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

[1.30 beta] Regression in cfg attribute parsing #54463

Closed
pietroalbini opened this issue Sep 22, 2018 · 6 comments
Closed

[1.30 beta] Regression in cfg attribute parsing #54463

pietroalbini opened this issue Sep 22, 2018 · 6 comments
Assignees
Labels
A-parser Area: The parsing of Rust source code to an AST regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Milestone

Comments

@pietroalbini
Copy link
Member

Parsing of the following snippet is failing on Rust 1.30 beta:

#![cfg_attr(feature = "nightly", feature(plugin_registrar, rustc_private,quote,),)]

cc @petrochenkov

@pietroalbini pietroalbini added A-parser Area: The parsing of Rust source code to an AST I-nominated T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. regression-from-stable-to-beta Performance or correctness regression from stable to beta. labels Sep 22, 2018
@pietroalbini pietroalbini added this to the 1.30 milestone Sep 22, 2018
@petrochenkov
Copy link
Contributor

Most probably caused by #53893.
I wonder why the previous crater run didn't caught this.

@petrochenkov petrochenkov self-assigned this Sep 22, 2018
@eddyb
Copy link
Member

eddyb commented Sep 22, 2018

@petrochenkov According to @pietroalbini, before and after had the same error (for #53893).
Is it possible a (different?) regression landed on master before that crater run?

@pietroalbini
Copy link
Member Author

The regression is caused by #53293, which didn't go through a Crater run.

@petrochenkov
Copy link
Contributor

(plugin_registrar, rustc_private,quote,), after the attribute path feature was parsed as an arbitrary token stream containing two token trees - parenthesized group (...) and comma ,.

Arbitrary token streams were never supposed to work on stable channel, but this wasn't properly enforced.
In particular, the unstable syntax was usable in code that's cfg-ed out.
The point of #53293 was exactly to fix this hole, so I think this issue is WONTFIX, as stated by rust-lang/rfcs#2405, especially given that this is the only regression of this kind and the code in command-macros was fixed long ago.

Note, that #![cfg_attr(feature = "nightly", feature(plugin_registrar, rustc_private,quote,),)] doesn't compile on stable if feature = "nightly" is actually enabled.

@petrochenkov
Copy link
Contributor

We can accept trailing comma in cfg_attr specifically though.
It will need to be accepted anyway when rust-lang/rfcs#2539 is implemented.

@petrochenkov
Copy link
Contributor

Fixed in #54581

kennytm added a commit to kennytm/rust that referenced this issue Sep 27, 2018
Accept trailing comma in `cfg_attr`

Fixes rust-lang#54463 (stable-to-beta regression)
bors added a commit that referenced this issue Sep 27, 2018
Accept trailing comma in `cfg_attr`

Fixes #54463 (stable-to-beta regression)
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 regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants