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

rust-2021-incompatible-closure-captures fires in 2021 edition code #101284

Closed
lnicola opened this issue Sep 1, 2022 · 3 comments · Fixed by #101409
Closed

rust-2021-incompatible-closure-captures fires in 2021 edition code #101284

lnicola opened this issue Sep 1, 2022 · 3 comments · Fixed by #101409
Assignees
Labels
A-closures Area: Closures (`|…| { … }`) A-diagnostics Area: Messages for errors, warnings, and lints A-edition-2021 Area: The 2021 edition A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. D-edition Diagnostics: An error or lint that should account for edition differences. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@lnicola
Copy link
Member

lnicola commented Sep 1, 2022

Given the following code: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=92d18abb330e3fbc8a1425f3458830da

#![warn(rust_2021_compatibility)]
use std::sync::Arc;

pub struct Warns {
    _test: Arc<String>, // Removing this Arc results in no warning
    foo: String,
}

impl Warns {
    pub fn test(self) -> std::io::Result<()> {
        let closure = move || {
            let _ = self.foo;
            Ok(())
        };
        closure()
    }
}

The current output is:

warning: changes to closure capture in Rust 2021 will affect drop order
  --> src/lib.rs:11:23
   |
11 |         let closure = move || {
   |                       ^^^^^^^
12 |             let _ = self.foo;
   |                     ---- in Rust 2018, this causes the closure to capture `self`, but in Rust 2021, it has no effect
...
16 |     }
   |     - in Rust 2018, `self` is dropped here along with the closure, but in Rust 2021 `self` is not part of the closure
   |
note: the lint level is defined here
  --> src/lib.rs:1:9
   |
1  | #![warn(rust_2021_compatibility)]
   |         ^^^^^^^^^^^^^^^^^^^^^^^
   = note: `#[warn(rust_2021_incompatible_closure_captures)]` implied by `#[warn(rust_2021_compatibility)]`
   = note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2021/disjoint-capture-in-closures.html>
help: add a dummy let to cause `self` to be fully captured
   |
11 ~         let closure = move || {
12 +             let _ = &self;
   |

warning: `playground` (lib) generated 1 warning
    Finished dev [unoptimized + debuginfo] target(s) in 0.52s

Ideally the output should look like:

[no warnings]

The code is already being compiled for the 2021 edition, there's no need to issue any warnings about behaviour that was different in 2018.
The compiler should assume that the code is working as expected.

This might affect other warnings in the same category, I'm not sure.

Tested in 1.63, beta and nightly.

@lnicola lnicola added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 1, 2022
@estebank estebank added A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. D-edition Diagnostics: An error or lint that should account for edition differences. A-edition-2021 Area: The 2021 edition A-closures Area: Closures (`|…| { … }`) labels Sep 1, 2022
@WaffleLapkin
Copy link
Member

@rustbot claim

@WaffleLapkin
Copy link
Member

This might affect other warnings in the same category, I'm not sure.

So, it seems that there are 3 warnings in this category and the issue only affects the closure one:

  • array_into_iter is not affected because it checks that auto-ref happens, which doesn't in rust 2021
  • non_fmt_panics is not affected because the linted code simply doesn't compile in rust 2021
  • rust_2021_incompatible_closure_captures is affected

@WaffleLapkin
Copy link
Member

Arc seems to be needed, because Vec is marked with #[rustc_insignificant_dtor] and the lint wants a significant Drop impl 😅

MRE:

#![warn(rust_2021_compatibility)]

pub struct Warns {
    // `Arc` has significant drop
    _significant_drop: std::sync::Arc<()>,
    field: String,
}

pub fn test(w: Warns) {
    _ = || drop(w.field);
}

fn main() {}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-closures Area: Closures (`|…| { … }`) A-diagnostics Area: Messages for errors, warnings, and lints A-edition-2021 Area: The 2021 edition A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. D-edition Diagnostics: An error or lint that should account for edition differences. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants