Skip to content

Refactor: Remove unused variable assignments for fallible functions #8247

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

Merged
merged 3 commits into from
Apr 17, 2025

Conversation

andrzejSulkowski
Copy link
Contributor

Description

This PR resolves issue #8236.

I recently graduated from PBA, and during one of our sessions, @shawntabrizi pointed out an important issue related to error handling in Rust.

When using let _ = some_fallible_function();, if the result is not followed by a ?, the error is silently swallowed without any warning or compiler feedback.

In contrast, if we don’t use let _ = and forget to add a ?, the compiler will correctly emit a warning or error — helping the developer catch the issue early.

This behavior can easily lead to bugs going unnoticed and makes error handling less reliable, especially for beginners following examples.

Integration

This PR introduces no functional or logical changes, and therefore can safely be integrated into existing downstream projects without additional adjustments.

From my point of view, this issue can be classified as something like I4-Silent.

Review Notes

I went through all occurrences of let _ = with a fallible function. Some of them return values tagged as #[must_use]. In these cases, I retained the underscore operator intentionally (see: polkadot/node/core/av-store/src/lib.rs lines: 1099 & 1108, polkadot/xcm/xcm-builder/src/currency_adapter.rs line: 217, substrate/frame/contracts/src/wasm/mod.rs line: 360, substrate/frame/revive/src/wasm/mod.rs line: 307).

@andrzejSulkowski andrzejSulkowski requested review from cheme, a team and koute as code owners April 15, 2025 10:52
@cla-bot-2021
Copy link

cla-bot-2021 bot commented Apr 15, 2025

User @andrzejSulkowski, please sign the CLA here.

@serban300 serban300 added R0-silent The change does not warrant a re-release of the modified crates. I9-optimisation An enhancement to provide better overall performance in terms of time-to-completion for a task. labels Apr 15, 2025
@bkchr bkchr removed the I9-optimisation An enhancement to provide better overall performance in terms of time-to-completion for a task. label Apr 17, 2025
@bkchr
Copy link
Member

bkchr commented Apr 17, 2025

@andrzejSulkowski please sign the CLA.

@andrzejSulkowski
Copy link
Contributor Author

@bkchr signed

@bkchr bkchr merged commit d3128dc into paritytech:master Apr 17, 2025
230 of 243 checks passed
Krayt78 pushed a commit to Krayt78/polkadot-sdk that referenced this pull request Apr 18, 2025
…aritytech#8247)

# Description
This PR resolves issue paritytech#8236.

I recently graduated from PBA, and during one of our sessions,
[@shawntabrizi](https://github.com/shawntabrizi) pointed out an
important issue related to error handling in Rust.

When using let _ = some_fallible_function();, if the result is not
followed by a ?, the error is silently swallowed without any warning or
compiler feedback.

In contrast, if we don’t use let _ = and forget to add a ?, the compiler
will correctly emit a warning or error — helping the developer catch the
issue early.

This behavior can easily lead to bugs going unnoticed and makes error
handling less reliable, especially for beginners following examples.


## Integration
This PR introduces no functional or logical changes, and therefore can
safely be integrated into existing downstream projects without
additional adjustments.

From my point of view, this issue can be classified as something like
`I4-Silent`.

## Review Notes

I went through all occurrences of `let _ =` with a fallible function.
Some of them return values tagged as `#[must_use]`. In these cases, I
retained the underscore operator intentionally _(see:
`polkadot/node/core/av-store/src/lib.rs` lines: 1099 & 1108,
`polkadot/xcm/xcm-builder/src/currency_adapter.rs` line: 217,
`substrate/frame/contracts/src/wasm/mod.rs` line: 360,
`substrate/frame/revive/src/wasm/mod.rs` line: 307)_.

Co-authored-by: Shawn Tabrizi <[email protected]>
Co-authored-by: Bastian Köcher <[email protected]>
castillax pushed a commit that referenced this pull request May 12, 2025
…8247)

# Description
This PR resolves issue #8236.

I recently graduated from PBA, and during one of our sessions,
[@shawntabrizi](https://github.com/shawntabrizi) pointed out an
important issue related to error handling in Rust.

When using let _ = some_fallible_function();, if the result is not
followed by a ?, the error is silently swallowed without any warning or
compiler feedback.

In contrast, if we don’t use let _ = and forget to add a ?, the compiler
will correctly emit a warning or error — helping the developer catch the
issue early.

This behavior can easily lead to bugs going unnoticed and makes error
handling less reliable, especially for beginners following examples.


## Integration
This PR introduces no functional or logical changes, and therefore can
safely be integrated into existing downstream projects without
additional adjustments.

From my point of view, this issue can be classified as something like
`I4-Silent`.

## Review Notes

I went through all occurrences of `let _ =` with a fallible function.
Some of them return values tagged as `#[must_use]`. In these cases, I
retained the underscore operator intentionally _(see:
`polkadot/node/core/av-store/src/lib.rs` lines: 1099 & 1108,
`polkadot/xcm/xcm-builder/src/currency_adapter.rs` line: 217,
`substrate/frame/contracts/src/wasm/mod.rs` line: 360,
`substrate/frame/revive/src/wasm/mod.rs` line: 307)_.

Co-authored-by: Shawn Tabrizi <[email protected]>
Co-authored-by: Bastian Köcher <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R0-silent The change does not warrant a re-release of the modified crates.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants