Skip to content

add #[must_use] to warn careless users of accidentally ignoring its call. #286

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

Closed
owtotwo opened this issue Apr 22, 2025 · 3 comments
Closed

Comments

@owtotwo
Copy link
Contributor

owtotwo commented Apr 22, 2025

According to Rust official developer recommendations, we can add for some struct, enum, trait or function. But its description is a bit vague, so it is difficult to specify exactly where to add #[must_use].

We need a concise rule that does not give false positives, to serve as a rule for adding #[must_use].

If some functions are added but others are omitted, then this is an ERROR, so I proposed in the PR #284 to first remove the use of #[must_use], and then mark all the missing functions after determining the rules.

Here are some discussions on how to make this rule:

We can observe how must_use is used in different code bases. As listed above, the standard library has some places using it, while the official hashbrown project rarely uses it:

My brief opinion is:

  1. For pure functions, whenever they are called, their return values ​​will be used, so mark them;
  2. For functions with side effects, we do not mark them by default;
  3. But if their return values ​​are likely to be used in common use cases, then we add them;
  4. For auxiliary structures(or enum and trait), they are often used as return values ​​(such as the mark on core::iter::Map), if the usage is similar, then also mark it;
  5. The rest are not marked for the time being. If there is a special use case, you can submit a PR and explain the reason for adding it. (Of course, you can also attach the reason to #[must_use = "<the reason>"])
@owtotwo
Copy link
Contributor Author

owtotwo commented Apr 22, 2025

@yegor256 I will first draft a PR based on this rule.
And I may have missed something. If so, please feel free to provide additional comments. :)

@owtotwo
Copy link
Contributor Author

owtotwo commented May 11, 2025

Cause #287 is merged but the patch version is not bumped (It should be, for adding #[must_use] is a breaking change.), we have not yet fully resolved this issue.

So keep it open for now.

@owtotwo
Copy link
Contributor Author

owtotwo commented May 11, 2025

Before we updated to 0.1.0, we run the cargo semver-checks would fail cause 0.0.19 should be bump to 0.0.20 or 0.1.0 (for adding #[must_use] is a breaking change).

But now we successes to check it:

~\..\..\micromap (master ≣ +0 ~2 -0 !) > cargo semver-checks
    Building micromap v0.1.0 (current)
       Built [   2.432s] (current)
     Parsing micromap v0.1.0 (current)
      Parsed [   0.010s] (current)
     Parsing micromap v0.0.19 (baseline, cached)
      Parsed [   0.027s] (baseline)
    Checking micromap v0.0.19 -> v0.1.0 (major change)
     Checked [   0.000s] 0 checks: 0 pass, 164 skip
     Summary no semver update required
    Finished [   3.114s] micromap

So this issue is fixed!

@owtotwo owtotwo closed this as completed May 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant