Skip to content

Lint request: warn when temporaries are used for values for which dropping has side effects #1904

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

Open
PieterPenninckx opened this issue Jul 26, 2017 · 4 comments
Labels
E-medium Call for participation: Medium difficulty level problem and requires some initial experience. L-correctness Lint: Belongs in the correctness lint group T-middle Type: Probably requires verifiying types T-MIR Type: This lint will require working with the MIR

Comments

@PieterPenninckx
Copy link

PieterPenninckx commented Jul 26, 2017

Temporary values (that I tend to call 'anonymous values') are values for which there is no variable binding. E.g.:

let a = Vec::<u8>::new().len();

Here the value of the type Vec<u8> is a temporary value.

Temporary values are -- generally speaking -- dropped at the semi-colon that terminates the statement in which they were created. More details can be found in the reference. This is not so well known and can be confusing, see e.g. rust-lang/book#1871. This can lead to bugs when a temporary is used to store a value for which the precise point where it is dropped is important, such as a LockResult. An example can be found in rust-lang/rust#37612.

To prevent such bugs, I think it would be a good idea if clippy would warn when a 'sensitive' value is stored in a temporary. With a 'sensitive' value, I mean something like a LockResult where calling the drop() has important side effects (so not just releasing memory).

@PieterPenninckx
Copy link
Author

I realized that my request was too vague about which datatypes have important side effects when being dropped. Since there is no way clippy could judge this, I would just work with a list. This will not capture all problems, but hopefully enough of them. This is the list I came up with:

  • std::sync::LockResult<T>
  • std::sync::TryLockResult<T>
  • std::sync::MutexGuard<T>
  • std::sync::RwLockReadGuard<T>
  • std::sync::RwLockWriteGuard<T>
  • std::cell::Ref<T>
  • std::cell::RefMut<T>

@oli-obk oli-obk added L-correctness Lint: Belongs in the correctness lint group E-medium Call for participation: Medium difficulty level problem and requires some initial experience. T-middle Type: Probably requires verifiying types T-MIR Type: This lint will require working with the MIR labels Jul 27, 2017
@PieterPenninckx
Copy link
Author

PieterPenninckx commented Jul 1, 2018

I'm working on this.
Update: I gave up a long time ago.

@academician
Copy link

Is it worth revisiting and possibly closing this issue after the changes to temporary scope in the 2024 edition?

@PieterPenninckx
Copy link
Author

@academician , I think the issue is still there in the 2024 edition: the 2024 edition only narrowed the scope when the temporary is in a tail expression, which is a case I hadn't ever considered. If the temporary is not in a tail expression, the issue is still there.

Also, reading into the documentation again after seven years, I now read "The exact rules for temporary lifetime extension are subject to change.", which to me is a sign that storing something of which the drop order is of importance in a temporary can still lead to issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-medium Call for participation: Medium difficulty level problem and requires some initial experience. L-correctness Lint: Belongs in the correctness lint group T-middle Type: Probably requires verifiying types T-MIR Type: This lint will require working with the MIR
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants