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

regression: proc-macro derive unparseable #85692

Closed
Mark-Simulacrum opened this issue May 25, 2021 · 6 comments
Closed

regression: proc-macro derive unparseable #85692

Mark-Simulacrum opened this issue May 25, 2021 · 6 comments
Labels
A-proc-macros Area: Procedural macros regression-from-stable-to-stable Performance or correctness regression from one stable version to another. relnotes Marks issues that should be documented in the release notes of the next release. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Milestone

Comments

@Mark-Simulacrum
Copy link
Member

From https://crater-reports.s3.amazonaws.com/beta-1.52-2/beta-2021-05-23/gh/omegablitz.gluon-bug/log.txt

[INFO] [stdout] error: expected one of `{`, lifetime, or type, found `,`
[INFO] [stdout]    --> /opt/rustwide/cargo-home/git/checkouts/gluon-9c68e1737197fd9b/77652ab/base/src/types/mod.rs:522:17
[INFO] [stdout]     |
[INFO] [stdout] 522 | #[derive(Clone, AstClone)]
[INFO] [stdout]     |                 ^^^^^^^^ unexpected token
[INFO] [stdout] ...
[INFO] [stdout] 543 |     T: TypePtr<Id = Id>,
[INFO] [stdout]     |                         - expected one of `{`, lifetime, or type
[INFO] [stdout]     |
[INFO] [stdout]     = note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)
[INFO] [stdout] 
[INFO] [stdout] 
[INFO] [stdout] error: proc-macro derive produced unparseable tokens
[INFO] [stdout]    --> /opt/rustwide/cargo-home/git/checkouts/gluon-9c68e1737197fd9b/77652ab/base/src/types/mod.rs:522:17
[INFO] [stdout]     |
[INFO] [stdout] 522 | #[derive(Clone, AstClone)]
[INFO] [stdout]     |                 ^^^^^^^^

cc @petrochenkov @Aaron1011 -- any recent parser changes you can think of that might've caused this?

@Mark-Simulacrum Mark-Simulacrum added 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. E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example labels May 25, 2021
@Mark-Simulacrum Mark-Simulacrum added this to the 1.53.0 milestone May 25, 2021
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label May 25, 2021
@jonas-schievink jonas-schievink added the A-proc-macros Area: Procedural macros label May 25, 2021
@Aaron1011
Copy link
Member

This appears to have already fixed in gluon-lang/gluon#889. I'm not sure why this is just now showing up again on Crater.

@Aaron1011
Copy link
Member

Oh, I see - the affected derive invocation also has a #[cfg_attr] attribute on the same attribute target. This previously meant that we would pretty-print and retokenize the input, which would cause the trailing comma to get lost.

This is expected fallout from #82608 - preserving the exact TokenStream may expose bugs in user-written proc macros. Fortunately, the bug in gluon_codegen is already fixed.

@Mark-Simulacrum Mark-Simulacrum added relnotes Marks issues that should be documented in the release notes of the next release. and removed E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels May 25, 2021
@Mark-Simulacrum
Copy link
Member Author

Tagging as relnotes (cc @XAMPPRocky) and removing prioritization request; I expect to close as won't fix with a note that duplicate trailing commas are no longer accepted placed in the compatibility notes.

@Mark-Simulacrum
Copy link
Member Author

@Aaron1011 is https://crater-reports.s3.amazonaws.com/beta-1.52-2/beta-2021-05-23/gh/yejingchen.jieba-bot/log.txt (source code in https://docs.rs/crate/tbot/0.6.7/source/src/errors/https_webhook.rs) basically the same error as here? It looks like it's parsing the Path, as a tuple perhaps -- seems a bit different.

@Aaron1011
Copy link
Member

@Mark-Simulacrum: Yes, the underlying cause (exposed bug due to preserving tokens when #[cfg_attr] is involved) is the same. That bug also has a fix merged in kdy1/is-macro#8.

@LeSeulArtichaut
Copy link
Contributor

I expect to close as won't fix with a note that duplicate trailing commas are no longer accepted placed in the compatibility notes.

Should we close this as wontfix now that 1.53.0 is released?

@LeSeulArtichaut LeSeulArtichaut added regression-from-stable-to-stable Performance or correctness regression from one stable version to another. and removed regression-from-stable-to-beta Performance or correctness regression from stable to beta. labels Jun 22, 2021
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Jun 22, 2021
@LeSeulArtichaut LeSeulArtichaut removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Jun 22, 2021
SnejUgal added a commit to tbot-rs/tbot that referenced this issue Jul 4, 2021
Rust 1.53 introduced an intentional breaking change in attribute
handling in macros, and it led to `is-macro` incorrectly generating code
for `errors::HttpsWebhook::Tls`
(investigated in [rust-lang/rust#85692]). A fix for that landed in
`is-macro` 0.1.9 via [kdy1/is-macro#8], so `tbot` needs to require
`is-macro` to be of version `^0.1.9`.

[rust-lang/rust#85692]: rust-lang/rust#85692
[kdy1/is-macro#8]: kdy1/is-macro#8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-proc-macros Area: Procedural macros regression-from-stable-to-stable Performance or correctness regression from one stable version to another. relnotes Marks issues that should be documented in the release notes of the next release. 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

6 participants