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.

Sign up for GitHub

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

Jump to bottom

Error when proc macro derive output doesn't fully parse #87316

Closed
wants to merge 4 commits into from
Closed

Error when proc macro derive output doesn't fully parse #87316

wants to merge 4 commits into from

Conversation

m-ou-se
Copy link
Member

@m-ou-se m-ou-se commented Jul 20, 2021

See #87314

Fixes #87314

@rust-highfive
Copy link
Collaborator

r? @matthewjasper

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 20, 2021
@m-ou-se m-ou-se 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 A-proc-macros Area: Procedural macros 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 20, 2021
@eddyb
Copy link
Member

eddyb commented Jul 20, 2021

r? @petrochenkov

@jonas-schievink jonas-schievink mentioned this pull request Jul 20, 2021
Open
@petrochenkov
Copy link
Contributor

@bors try

@petrochenkov petrochenkov added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 20, 2021
@bors
Copy link
Contributor

bors commented Jul 20, 2021

⌛ Trying commit 2bd0a21321c4908e865e641fee5e532f03c37788 with merge 279ba2cff36a3029611d32ee1b5c82559c565f69...

@bors
Copy link
Contributor

bors commented Jul 20, 2021

☀️ Try build successful - checks-actions
Build commit: 279ba2cff36a3029611d32ee1b5c82559c565f69 (279ba2cff36a3029611d32ee1b5c82559c565f69)

@petrochenkov
Copy link
Contributor

@craterbot check

@craterbot
Copy link
Collaborator

👌 Experiment pr-87316 created and queued.
🤖 Automatically detected try build 279ba2cff36a3029611d32ee1b5c82559c565f69
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@m-ou-se m-ou-se force-pushed the fix-87314 branch 2 times, most recently from 80bf842 to 6024115 Compare July 28, 2021 13:41
@craterbot
Copy link
Collaborator

🚧 Experiment pr-87316 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot
Copy link
Collaborator

🎉 Experiment pr-87316 is completed!
📊 170 regressed and 9 fixed (173919 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@petrochenkov petrochenkov added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Aug 2, 2021
@m-ou-se
Copy link
Member Author

m-ou-se commented Aug 2, 2021

Not a lot of regressions. Haven't looked at all of them yet, but most of them seem to be about an extra ; in the output that was previously ignored and now no longer accepted:

We sould probably not error and produce a future-incompatible warning for an extra ; here.

@m-ou-se
Copy link
Member Author

m-ou-se commented Aug 5, 2021 •
edited
Loading

Analyzed the results now.

By far the most cases are from derive(JsonSchema) from schemars v0.8.0. But that problem has been fixed in v0.8.3 four months ago.

Two are in crates that have a very low number of downloads and haven't been updated in a long time.

The last few seem to be through prost-derive. The problem occurs in 0.5, but it's been fixed in 0.6 and above as of almost two years ago.

In all cases it's a semicolon at the end of the token stream, without anything following it. So no items got silently ignored.

@petrochenkov petrochenkov added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 5, 2021
@petrochenkov
Copy link
Contributor

With the number of regressions this large (>10) we've been typically adding deprecation lints in the past, the lint can be immediately made deny-by-default in this case, I think.

@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 5, 2021
@JohnCSimon JohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 22, 2021
@JohnCSimon JohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 6, 2021
@JohnCSimon JohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 27, 2021
@JohnCSimon
Copy link
Member

@m-ou-se What is the status of this PR?

@m-ou-se
Copy link
Member Author

m-ou-se commented Oct 19, 2021

I need to change this from a hard error to a lint. But I've not had time for that yet.

@JohnCSimon JohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 8, 2021
@JohnCSimon JohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 5, 2021
@JohnCSimon JohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 30, 2022
@JohnCSimon JohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 27, 2022
@bors
Copy link
Contributor

bors commented Mar 15, 2022

☔ The latest upstream changes (presumably #94584) made this pull request unmergeable. Please resolve the merge conflicts.

@camelid camelid added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 2, 2022
@JohnCSimon
Copy link
Member

@m-ou-se
Ping from triage: I'm closing this due to inactivity, Please reopen when you are ready to continue with this.
Note: if you do please open the PR BEFORE you push to it, else you won't be able to reopen - this is a quirk of github.
Thanks for your contribution.

@rustbot label: +S-inactive

@JohnCSimon JohnCSimon closed this Nov 27, 2022
@rustbot rustbot added the S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. label Nov 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Aaron1011 Aaron1011 Aaron1011 left review comments

@mystor mystor mystor left review comments

Assignees

@petrochenkov petrochenkov

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 A-proc-macros Area: Procedural macros C-bug Category: This is a bug. S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Invalid derive proc macro output silently ignored