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

Pretty-printing/tokenization of final has changed to r#final #62628

Closed
alexcrichton opened this issue Jul 12, 2019 · 6 comments · Fixed by #62667
Closed

Pretty-printing/tokenization of final has changed to r#final #62628

alexcrichton opened this issue Jul 12, 2019 · 6 comments · Fixed by #62667
Assignees
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-parser Area: The parsing of Rust source code to an AST. C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@alexcrichton
Copy link
Member

Given a procedural macro like:

extern crate proc_macro;

use proc_macro::*;

#[proc_macro_attribute]
pub fn foo(a: TokenStream, b: TokenStream) -> TokenStream {
    assert_eq!(a.to_string(), "final");
    return b.into_iter().collect();
}

and a consumer like:

#[foo::foo(final)]
fn main() {
    #[foo::foo(final)]
    fn foo() {}
}

This previously compiled and succeeded on nightly-2019-07-10 but fails on nightly-2017-07-11. Bisection yields #62555 as the culprit of which #62393 is the likely true culprit.

When running if you set RUSTC_LOG=syntax you get:

 INFO 2019-07-12T16:54:49Z: syntax::parse::token: cached tokens found, but they're not "probably equal", going with stringified version

which I believe means that the compiler is pretty-printing the identifier in macros as r#final instead of final.

For reference this is a pretty minimized scenario but it has broken wasm-bindgen's test suite (which uses #[wasm_bindgen(final)] throughout). @petrochenkov is this intentional? Is this something that should be fixed or "the macro parser needs to be update"?

(I'm gonna update the parser anyway but wanted to be sure to file an issue as well)

@Mark-Simulacrum
Copy link
Member

I think we probably threaded through a is_raw_guess somewhere we didn't before. I think it's this code: https://github.com/rust-lang/rust/pull/62393/files#diff-c82811a081b5fb219ba68dd88b49d0b1R670. Previously we'd just directly stringify so r# wasn't added but now we try to guess.

alexcrichton added a commit to alexcrichton/wasm-bindgen that referenced this issue Jul 12, 2019
This fixes an issue also reported to upstream (rust-lang/rust#62628) to
ensure that we parse the `final` attribute as either `r#final` or
`final`, since now the compiler is giving us `r#final` and we were
previously only accepting `final`.

The parsing here was a bit wonky, but this setup ended up working!
@petrochenkov
Copy link
Contributor

petrochenkov commented Jul 12, 2019

This looks like a bug, I assumed that print_attribute_path is used only for the attribute path foo::foo, but not for the attribute arguments (final) which are treated as tokens, but somehow it gets to the arguments as well, which is not right.

@petrochenkov petrochenkov self-assigned this Jul 12, 2019
@jonas-schievink jonas-schievink added A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-parser Area: The parsing of Rust source code to an AST. C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 12, 2019
@petrochenkov
Copy link
Contributor

I'll look into more detail today.
(Perhaps some attributes go through a conversion to MetaItem and back unnecessarily?)

@petrochenkov
Copy link
Contributor

Perhaps some attributes go through a conversion to MetaItem and back unnecessarily?

This is indeed the case and it's trivially fixable, but the trivial fix regresses formatting of the pretty-printed code.
I'm currently trying to pick the right combination of iboxes and cboxes for the things to look good again, will submit a PR soon.

@petrochenkov
Copy link
Contributor

Fixed in #62667.

petrochenkov added a commit to petrochenkov/rust that referenced this issue Jul 15, 2019
bors added a commit that referenced this issue Jul 15, 2019
pprust: Improve pretty-printing of delimited token groups

The commit "Do not convert attributes into `MetaItem`s for printing" fixes #62628.

Other commits fix regressions from abandoning `MetaItem`s, and make formatting for attributes, macro calls, macro definitions and other delimited token groups better and more consistent.

r? @Mark-Simulacrum
@alexcrichton
Copy link
Member Author

Thanks for the quick fix @petrochenkov!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-parser Area: The parsing of Rust source code to an AST. C-bug Category: This is a bug. 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