Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use bool::then instead of then_some with function calls #6156

Merged
merged 6 commits into from
Oct 22, 2024

Conversation

tmpolaczyk
Copy link
Contributor

@tmpolaczyk tmpolaczyk commented Oct 21, 2024

I noticed that hardware benchmarks are being run even though we pass the --no-hardware-benchmarks cli flag. After some debugging, the cause is an incorrect usage of the then_some method.

From std docs:

Arguments passed to then_some are eagerly evaluated; if you are passing the result of a function call, it is recommended to use then, which is lazily evaluated.

let mut a = 0;
let mut function_with_side_effects = || { a += 1; };

true.then_some(function_with_side_effects());
false.then_some(function_with_side_effects());

// `a` is incremented twice because the value passed to `then_some` is
// evaluated eagerly.
assert_eq!(a, 2);

This PR fixes all the similar usages of the then_some method across the codebase.

polkadot address: 138eUqXvUYT3o4GdbnWQfGRzM8yDWh5Q2eFrFULL7RAXzdWD

@tmpolaczyk tmpolaczyk requested a review from a team as a code owner October 21, 2024 11:02
@@ -173,7 +173,7 @@ impl<B: BlockT> StateStrategy<B> {
peer_id: PeerId,
announce: &BlockAnnounce<B::Header>,
) -> Option<(B::Hash, NumberFor<B>)> {
is_best.then_some({
is_best.then(|| {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is pretty bad, it makes on_validated_block_announce behave as if is_best was always true

Copy link
Member

@ggwpez ggwpez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huge facepalm moment, thanks for fixing!

Copy link

Command failed ❌

Run by @ggwpez for Command PrDoc failed. See logs here.

@shawntabrizi
Copy link
Member

/tip small

Copy link

@tmpolaczyk Contributor did not properly post their account address.

Make sure the pull request description (or user bio) has: "{network} address: {address}".

@shawntabrizi shawntabrizi added the T12-benchmarks This PR/Issue is related to benchmarking and weights. label Oct 21, 2024
shawntabrizi and others added 3 commits October 21, 2024 11:46
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
@ggwpez
Copy link
Member

ggwpez commented Oct 21, 2024

@paritytech/smart-contracts PTAL - CI needs your approval

@bkchr bkchr disabled auto-merge October 22, 2024 20:23
@bkchr bkchr merged commit 6418131 into paritytech:master Oct 22, 2024
192 of 196 checks passed
@shawntabrizi
Copy link
Member

/tip small

Copy link

@shawntabrizi A referendum for a small (20 DOT) tip was successfully submitted for @tmpolaczyk (138eUqXvUYT3o4GdbnWQfGRzM8yDWh5Q2eFrFULL7RAXzdWD on polkadot).

Referendum number: 1251.
tip

Copy link

The referendum has appeared on Polkassembly.

mordamax pushed a commit to paritytech-stg/polkadot-sdk that referenced this pull request Oct 25, 2024
)

I noticed that hardware benchmarks are being run even though we pass the
--no-hardware-benchmarks cli flag. After some debugging, the cause is an
incorrect usage of the `then_some` method.

From [std
docs](https://doc.rust-lang.org/std/primitive.bool.html#method.then_some):

> Arguments passed to then_some are eagerly evaluated; if you are
passing the result of a function call, it is recommended to use
[then](https://doc.rust-lang.org/std/primitive.bool.html#method.then),
which is lazily evaluated.

```rust
let mut a = 0;
let mut function_with_side_effects = || { a += 1; };

true.then_some(function_with_side_effects());
false.then_some(function_with_side_effects());

// `a` is incremented twice because the value passed to `then_some` is
// evaluated eagerly.
assert_eq!(a, 2);
```

This PR fixes all the similar usages of the `then_some` method across
the codebase.

polkadot address: 138eUqXvUYT3o4GdbnWQfGRzM8yDWh5Q2eFrFULL7RAXzdWD

---------

Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Co-authored-by: Shawn Tabrizi <[email protected]>
Co-authored-by: Oliver Tale-Yazdi <[email protected]>
mordamax pushed a commit to paritytech-stg/polkadot-sdk that referenced this pull request Oct 25, 2024
)

I noticed that hardware benchmarks are being run even though we pass the
--no-hardware-benchmarks cli flag. After some debugging, the cause is an
incorrect usage of the `then_some` method.

From [std
docs](https://doc.rust-lang.org/std/primitive.bool.html#method.then_some):

> Arguments passed to then_some are eagerly evaluated; if you are
passing the result of a function call, it is recommended to use
[then](https://doc.rust-lang.org/std/primitive.bool.html#method.then),
which is lazily evaluated.

```rust
let mut a = 0;
let mut function_with_side_effects = || { a += 1; };

true.then_some(function_with_side_effects());
false.then_some(function_with_side_effects());

// `a` is incremented twice because the value passed to `then_some` is
// evaluated eagerly.
assert_eq!(a, 2);
```

This PR fixes all the similar usages of the `then_some` method across
the codebase.

polkadot address: 138eUqXvUYT3o4GdbnWQfGRzM8yDWh5Q2eFrFULL7RAXzdWD

---------

Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Co-authored-by: Shawn Tabrizi <[email protected]>
Co-authored-by: Oliver Tale-Yazdi <[email protected]>
mordamax pushed a commit to paritytech-stg/polkadot-sdk that referenced this pull request Oct 25, 2024
)

I noticed that hardware benchmarks are being run even though we pass the
--no-hardware-benchmarks cli flag. After some debugging, the cause is an
incorrect usage of the `then_some` method.

From [std
docs](https://doc.rust-lang.org/std/primitive.bool.html#method.then_some):

> Arguments passed to then_some are eagerly evaluated; if you are
passing the result of a function call, it is recommended to use
[then](https://doc.rust-lang.org/std/primitive.bool.html#method.then),
which is lazily evaluated.

```rust
let mut a = 0;
let mut function_with_side_effects = || { a += 1; };

true.then_some(function_with_side_effects());
false.then_some(function_with_side_effects());

// `a` is incremented twice because the value passed to `then_some` is
// evaluated eagerly.
assert_eq!(a, 2);
```

This PR fixes all the similar usages of the `then_some` method across
the codebase.

polkadot address: 138eUqXvUYT3o4GdbnWQfGRzM8yDWh5Q2eFrFULL7RAXzdWD

---------

Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Co-authored-by: Shawn Tabrizi <[email protected]>
Co-authored-by: Oliver Tale-Yazdi <[email protected]>
mordamax pushed a commit to paritytech-stg/polkadot-sdk that referenced this pull request Oct 25, 2024
)

I noticed that hardware benchmarks are being run even though we pass the
--no-hardware-benchmarks cli flag. After some debugging, the cause is an
incorrect usage of the `then_some` method.

From [std
docs](https://doc.rust-lang.org/std/primitive.bool.html#method.then_some):

> Arguments passed to then_some are eagerly evaluated; if you are
passing the result of a function call, it is recommended to use
[then](https://doc.rust-lang.org/std/primitive.bool.html#method.then),
which is lazily evaluated.

```rust
let mut a = 0;
let mut function_with_side_effects = || { a += 1; };

true.then_some(function_with_side_effects());
false.then_some(function_with_side_effects());

// `a` is incremented twice because the value passed to `then_some` is
// evaluated eagerly.
assert_eq!(a, 2);
```

This PR fixes all the similar usages of the `then_some` method across
the codebase.

polkadot address: 138eUqXvUYT3o4GdbnWQfGRzM8yDWh5Q2eFrFULL7RAXzdWD

---------

Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Co-authored-by: Shawn Tabrizi <[email protected]>
Co-authored-by: Oliver Tale-Yazdi <[email protected]>
mordamax pushed a commit to paritytech-stg/polkadot-sdk that referenced this pull request Oct 25, 2024
)

I noticed that hardware benchmarks are being run even though we pass the
--no-hardware-benchmarks cli flag. After some debugging, the cause is an
incorrect usage of the `then_some` method.

From [std
docs](https://doc.rust-lang.org/std/primitive.bool.html#method.then_some):

> Arguments passed to then_some are eagerly evaluated; if you are
passing the result of a function call, it is recommended to use
[then](https://doc.rust-lang.org/std/primitive.bool.html#method.then),
which is lazily evaluated.

```rust
let mut a = 0;
let mut function_with_side_effects = || { a += 1; };

true.then_some(function_with_side_effects());
false.then_some(function_with_side_effects());

// `a` is incremented twice because the value passed to `then_some` is
// evaluated eagerly.
assert_eq!(a, 2);
```

This PR fixes all the similar usages of the `then_some` method across
the codebase.

polkadot address: 138eUqXvUYT3o4GdbnWQfGRzM8yDWh5Q2eFrFULL7RAXzdWD

---------

Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Co-authored-by: Shawn Tabrizi <[email protected]>
Co-authored-by: Oliver Tale-Yazdi <[email protected]>
mordamax pushed a commit to paritytech-stg/polkadot-sdk that referenced this pull request Oct 29, 2024
)

I noticed that hardware benchmarks are being run even though we pass the
--no-hardware-benchmarks cli flag. After some debugging, the cause is an
incorrect usage of the `then_some` method.

From [std
docs](https://doc.rust-lang.org/std/primitive.bool.html#method.then_some):

> Arguments passed to then_some are eagerly evaluated; if you are
passing the result of a function call, it is recommended to use
[then](https://doc.rust-lang.org/std/primitive.bool.html#method.then),
which is lazily evaluated.

```rust
let mut a = 0;
let mut function_with_side_effects = || { a += 1; };

true.then_some(function_with_side_effects());
false.then_some(function_with_side_effects());

// `a` is incremented twice because the value passed to `then_some` is
// evaluated eagerly.
assert_eq!(a, 2);
```

This PR fixes all the similar usages of the `then_some` method across
the codebase.

polkadot address: 138eUqXvUYT3o4GdbnWQfGRzM8yDWh5Q2eFrFULL7RAXzdWD

---------

Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Co-authored-by: Shawn Tabrizi <[email protected]>
Co-authored-by: Oliver Tale-Yazdi <[email protected]>
mordamax pushed a commit to paritytech-stg/polkadot-sdk that referenced this pull request Oct 29, 2024
)

I noticed that hardware benchmarks are being run even though we pass the
--no-hardware-benchmarks cli flag. After some debugging, the cause is an
incorrect usage of the `then_some` method.

From [std
docs](https://doc.rust-lang.org/std/primitive.bool.html#method.then_some):

> Arguments passed to then_some are eagerly evaluated; if you are
passing the result of a function call, it is recommended to use
[then](https://doc.rust-lang.org/std/primitive.bool.html#method.then),
which is lazily evaluated.

```rust
let mut a = 0;
let mut function_with_side_effects = || { a += 1; };

true.then_some(function_with_side_effects());
false.then_some(function_with_side_effects());

// `a` is incremented twice because the value passed to `then_some` is
// evaluated eagerly.
assert_eq!(a, 2);
```

This PR fixes all the similar usages of the `then_some` method across
the codebase.

polkadot address: 138eUqXvUYT3o4GdbnWQfGRzM8yDWh5Q2eFrFULL7RAXzdWD

---------

Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Co-authored-by: Shawn Tabrizi <[email protected]>
Co-authored-by: Oliver Tale-Yazdi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T12-benchmarks This PR/Issue is related to benchmarking and weights.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants