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

Async fn loses mut pattern when passed to attribute macro #60674

Closed
dtolnay opened this issue May 9, 2019 · 6 comments
Closed

Async fn loses mut pattern when passed to attribute macro #60674

dtolnay opened this issue May 9, 2019 · 6 comments

Comments

@dtolnay
Copy link
Member

@dtolnay dtolnay commented May 9, 2019

Cargo.toml

[package]
name = "repro"
version = "0.0.0"
edition = "2018"

[lib]
proc-macro = true

src/lib.rs

extern crate proc_macro;
use proc_macro::TokenStream;

#[proc_macro_attribute]
pub fn attr(_args: TokenStream, input: TokenStream) -> TokenStream {
    println!("{}", input);
    TokenStream::new()
}

src/main.rs

#[repro::attr]
async fn f(mut x: u8) {}

fn main() {}

Between nightly-2019-05-07 and nightly-2019-05-08, the mut pattern that should be part of the macro input went missing. a19cf18...cfdc84a

$ cargo +nightly-2019-05-07 check
   Compiling repro v0.0.0
async fn f(mut x: u8) { }

$ cargo +nightly-2019-05-08 check
   Compiling repro v0.0.0
async fn f(x: u8) { }

Originally reported by @kjvalencik in dtolnay/syn#632.
Mentioning @taiki-e @cramertj @davidtwco because #60535 looks relevant and is in the right commit range.

@kjvalencik
Copy link

@kjvalencik kjvalencik commented May 9, 2019

FYI, &mut is not lost.

@davidtwco
Copy link
Member

@davidtwco davidtwco commented May 9, 2019

Thanks for the report, I'll look into this.

@taiki-e
Copy link
Member

@taiki-e taiki-e commented May 9, 2019

Hmm, seems it was too early to remove mut with this timing:

// Remove mutability from arguments. If this is not a simple pattern,
// those arguments are replaced by `__argN`, so there is no need to do this.
if let PatKind::Ident(BindingMode::ByValue(mutability @ Mutability::Mutable), ..) =
&mut input.pat.node
{
assert!(is_simple_pattern);
*mutability = Mutability::Immutable;
}

@taiki-e
Copy link
Member

@taiki-e taiki-e commented May 9, 2019

This problem only occurs with by-value binding(and simple patterns), so...

async fn foo(mut x: A) { ... }

// desugaring (current master)
fn foo(x: A) -> ... { // removed mutability
    async move {
        let mut x = x;
        ...
    }
}

// Probably it should desugar like this
fn foo(__arg0: A) -> ... {
    async move {
        let mut x = __arg0;
        ...
    }
}

davidtwco added a commit to davidtwco/rust that referenced this issue May 9, 2019
This commit adds a regression test (with current broken behaviour) that
tests that `mut` patterns are not lost when provided as input to a proc macro.
@davidtwco
Copy link
Member

@davidtwco davidtwco commented May 9, 2019

I've filed #60676 with a fix.

@taiki-e
Copy link
Member

@taiki-e taiki-e commented May 9, 2019

@davidtwco Thanks for the fix!

Centril added a commit to Centril/rust that referenced this issue May 9, 2019
Fix async desugaring providing wrong input to procedural macros.

Fixes rust-lang#60674.

This PR fixes a minor oversight introduced by rust-lang#60535 where unused `mut` binding modes were removed from the arguments to an `async fn` (as they were added to the statement that we insert into the closure body). However, this meant that the input to procedural macros was incorrect. This removes that and instead fixes the `unused_mut` error that it avoided.

r? @cramertj
cc @taiki-e
@bors bors closed this in #60676 May 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

5 participants