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

Visibility tokens on types in extern { ... } blocks are no longer given to procedural macros #69315

Closed
alexcrichton opened this issue Feb 20, 2020 · 5 comments · Fixed by #69334
Assignees
Labels
A-parser Area: The parsing of Rust source code to an AST A-visibility Area: Visibility / privacy regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@alexcrichton
Copy link
Member

I had rustwasm/wasm-bindgen#2009 opened up on a project recently but this looks to be a regression I think. This regressed between nightly-2020-02-19 and nightly-2020-02-20, and bisecting further by commit reveals #69271 as the culprit. I'm not sure which of the PRs in that rollup though are the culprit here.

This can be reproduced by running cargo check from the crates/js-sys folder in the wasm-bindgen repository. Unfortunately this one probably isn't super easy to reproduce, so I'm hoping that someone more knowledgeable can hopefully take a look at this and have a better idea about what's going on.

There's definitely macros going on in #[wasm_bindgen] (the Memory type is synthesized by a macro), which leads me to think that #69211 may be the culprit? (cc @petrochenkov)

@jonas-schievink jonas-schievink added A-visibility Area: Visibility / privacy C-bug Category: This is a bug. I-nominated regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 20, 2020
@Centril Centril added A-parser Area: The parsing of Rust source code to an AST E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example and removed C-bug Category: This is a bug. labels Feb 20, 2020
@petrochenkov
Copy link
Contributor

The large parser PR from @Centril (#69194) could also cause something like this.

@petrochenkov petrochenkov self-assigned this Feb 20, 2020
@petrochenkov
Copy link
Contributor

I'll be able to look at this during the weekend, but it would be great if someone could come up with a minimal reproduction before that.

@alexcrichton
Copy link
Member Author

Ok, wanted to get a gut-check to see if it was known what was happening. Did some reduction and it looks like the macro no longer gets the pub token for the input type.

Given this input:

pub mod wasm {
    #[the_macro::the_macro]
    extern "C" {
        pub type Memory;
    }
}

fn main() {
    let _: wasm::Memory;
}

then with this macro:

extern crate proc_macro;

use proc_macro::*;

#[proc_macro_attribute]
pub fn the_macro(_attr: TokenStream, body: TokenStream) -> TokenStream {
    let g = match body.clone().into_iter().nth(2) {
        Some(TokenTree::Group(g)) => g,
        _ => panic!("unexpected input"),
    };
    let tokens = g.stream().into_iter().collect::<Vec<_>>();
    assert_eq!(tokens[0].to_string(), "pub");
    return body;
}

it fails with:

$ cargo +nightly build
   Compiling wut v0.1.0 (/home/alex/code/wut)
error: custom attribute panicked
 --> src/main.rs:2:5
  |
2 |     #[the_macro::the_macro]
  |     ^^^^^^^^^^^^^^^^^^^^^^^
  |
  = help: message: assertion failed: `(left == right)`
            left: `"type"`,
           right: `"pub"`

error: aborting due to previous error

error: could not compile `wut`.

To learn more, run the command again with --verbose.

On stable it yields the expected error[E0658]: extern types are experimental error which indicates that the pub token was fed to the macro.

If the #[wasm_bindgen] macro doesn't see the pub token it doesn't make the final result public, which is why the privacy error happened in the first place.

@alexcrichton alexcrichton changed the title Macro-expanded public struct no longer considered public Visibility tokens on types in extern { ... } blocks are no longer given to procedural macros Feb 20, 2020
@alexcrichton
Copy link
Member Author

(updated the issue title to reflect what appears to be the actual bug)

@Centril Centril self-assigned this Feb 20, 2020
@Centril Centril removed the E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example label Feb 20, 2020
@Centril
Copy link
Contributor

Centril commented Feb 21, 2020

Fixed by #69334.

bors added a commit that referenced this issue Feb 22, 2020
print vis & defaultness for nested items

Fixes #69315 which was injected by #69194.

r? @petrochenkov
cc @alexcrichton
@bors bors closed this as completed in 1484196 Feb 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-parser Area: The parsing of Rust source code to an AST A-visibility Area: Visibility / privacy regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. 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.

4 participants