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

2021 disjoint capture suggestion breaks macro_rules macro #88440

Open
ehuss opened this issue Aug 28, 2021 · 3 comments
Open

2021 disjoint capture suggestion breaks macro_rules macro #88440

ehuss opened this issue Aug 28, 2021 · 3 comments
Labels
A-closures Area: closures (`|args| { .. }`) A-edition-2021 Area: The 2021 edition A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix`. C-bug Category: This is a bug. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code.

Comments

@ehuss
Copy link
Contributor

ehuss commented Aug 28, 2021

I tried this code:

#![allow(unused)]
#![warn(rust_2021_incompatible_closure_captures)]

macro_rules! foo {
    ($v:ident) => {
        move || {
            println!("{:?}", $v.0);
        }
    };
}

fn main() {
    let x = (vec![1], vec![2]);
    foo!(x);
}

The rust_2021_incompatible_closure_captures lint generates a suggestion which causes this to fail to compile:

macro_rules! foo {
    ($v:ident) => {
        move || {
            let _ = &x;  // ERROR: cannot find value `x` in this scope
            println!("{:?}", $v.0);
        }
    };
}

I'm not exactly sure what this should do. In this particular case, the suggestion is not required. However, that's a fundamental problem with the rust_2021_incompatible_closure_captures lint, as it does not know when it is required.

In general, modifying macros can be prone to failure, so maybe an alternative is to issue a warning with a non-machine-applicable fix when inside a macro_rules definition?

This was found in the 2021 crater run for rcodec 1.0.1: https://crater-reports.s3.amazonaws.com/pr-87190-3/try%23a7a572ce3edd6d476191fbfe92c9c1986e009b34/reg/rcodec-1.0.1/log.txt
Note that this has even worse behavior than the example above. The sample macro:

macro_rules! forcomp_stmts {
    { $yld:block, $v:ident, $e:expr } => {
        $e.map(move |$v| $yld)
    };
    { $yld:block, $v:ident, $e:expr, $($tv:ident, $te:expr),+ } => {
        $e.and_then(move |$v| {
            forcomp_stmts!($yld, $($tv, $te),+)
        })
    };
}

rewrites the map call to:

        $e.map{ let _ = &decoded; (move |$v| $yld) }

which is invalid syntax.

Meta

rustc --version --verbose:

rustc 1.56.0-nightly (ac50a5335 2021-08-27)
binary: rustc
commit-hash: ac50a53359328a5d7f2f558833e63d59d372e4f7
commit-date: 2021-08-27
host: x86_64-apple-darwin
release: 1.56.0-nightly
LLVM version: 13.0.0

cc @rust-lang/wg-rfc-2229

@ehuss ehuss added A-closures Area: closures (`|args| { .. }`) C-bug Category: This is a bug. A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix`. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. A-edition-2021 Area: The 2021 edition labels Aug 28, 2021
@arora-aman
Copy link
Member

arora-aman commented Aug 28, 2021

Some of my thought process:

My initial thoughts were we can just issue warnings for any closure that has a precise capture given it was defined within a macro.

Something like: Drop order/Trait might be affected, please check the edition guide on how to migrate. Without any specific suggestion and maybe we highlight macro a bit more in the edition guide.

But we can have a case where capture_from_closure!(some_struct.some_vec)

Hmm I suppose we will see this as precise capture.

I guess during typechk can we tell that a piece of code is with a macro? And can issue a warning that's worded appropriately.

@arora-aman
Copy link
Member

Maybe related to: #87955

@m-ou-se
Copy link
Member

m-ou-se commented Aug 31, 2021

rewrites the map call to:

        $e.map{ let _ = &decoded; (move |$v| $yld) }

which is invalid syntax.

The invalid syntax there is already fixed by #87958

image

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Sep 1, 2021
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Sep 1, 2021
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Sep 2, 2021
m-ou-se added a commit to m-ou-se/rust that referenced this issue Sep 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-closures Area: closures (`|args| { .. }`) A-edition-2021 Area: The 2021 edition A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix`. C-bug Category: This is a bug. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code.
Projects
None yet
Development

No branches or pull requests

3 participants