[CRATER] Crater rollup #130373
Closed
[CRATER] Crater rollup #130373
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
By using `token_descr`, as is done for many other errors, we can get slightly better descriptions in error messages, e.g. "macro expansion ignores token `let` and any following" becomes "macro expansion ignores keyword `let` and any tokens following". This will be more important once invisible delimiters start being mentioned in error messages -- without this commit, that leads to error messages such as "error at ``" because invisible delimiters are pretty printed as an empty string.
Much like the previous commit. I think the removal of "the token" in each message is fine here. There are many more error messages that mention tokens without saying "the token" than those that do say it.
It's not used meaningfully yet, but will be needed to get rid of interpolated tokens.
Pasted metavariables are wrapped in invisible delimiters, which pretty-print as empty strings, and changing that can break some proc macros. But error messages saying "expected identifer, found ``" are bad. So this commit adds support for metavariables in `TokenDescription` so they print as "metavariable" in error messages, instead of "``". It's not used meaningfully yet, but will be needed to get rid of interpolated tokens.
Current places where `Interpolated` is used are going to change to instead use invisible delimiters. This prepares for that. - It adds invisible delimiter cases to the `can_begin_*`/`may_be_*` methods and the `failed_to_match_macro` that are equivalent to the existing `Interpolated` cases. - It adds panics/asserts in some places where invisible delimiters should never occur. - In `Parser::parse_struct_fields` it excludes an ident + invisible delimiter from special consideration in an error message, because that's quite different to an ident + paren/brace/bracket.
We now use invisible delimiters for expanded `vis` fragments, instead of `Token::Interpolated`.
Notes about tests: - tests/ui/parser/macro/trait-object-macro-matcher.rs: the syntax error is duplicated, because it occurs now when parsing the decl macro input, and also when parsing the expanded decl macro. But this won't show up for normal users due to error de-duplication. - tests/ui/associated-consts/issue-93835.rs: ditto. - The changes to metavariable descriptions in this PR's earlier commits are now visible in error message for several tests.
The one notable test change is `tests/ui/macros/trace_faulty_macros.rs`. This commit removes the complicated `Interpolated` handling in `expected_expression_found` that results in a longer error message. But I think the new, shorter message is actually an improvement. The original complaint was in rust-lang#71039, when the error message started with "error: expected expression, found `1 + 1`". That was confusing because `1 + 1` is an expression. Other than that, the reporter said "the whole error message is not too bad if you ignore the first line". Subsequently, extra complexity and wording was added to the error message. But I don't think the extra wording actually helps all that much. In particular, it still says of the `1+1` that "this is expected to be expression". This repeats the problem from the original complaint! This commit removes the extra complexity, reverting to a simpler error message. This is primarily because the traversal a pain without `Interpolated` tokens. Nonetheless, I think the error message is *improved*. It now starts with "expected expression, found `pat` metavariable", which is much clearer and the real problem. It also doesn't say anything specific about `1+1`, which is good, because the `1+1` isn't really relevant to the error -- it's the `$e:pat` that's important.
This involves replacing `nt_pretty_printing_compatibility_hack` with `stream_pretty_printing_compatibility_hack`. The handling of statements in `transcribe` is slightly different to other nonterminal kinds, due to the lack of `from_ast` implementation for empty statements. Notable test changes: - `tests/ui/proc-macro/expand-to-derive.rs`: the diff looks large but the only difference is the insertion of a single invisible-delimited group around a metavar.
Note: there was an existing code path involving `Interpolated` in `MetaItem::from_tokens` that was dead. This commit transfers that to the new form, but puts an `unreachable!` call inside it.
Notes about tests: - tests/ui/rfcs/rfc-2294-if-let-guard/feature-gate.rs: some messages are now duplicated due to repeated parsing. - tests/ui/rfcs/rfc-2497-if-let-chains/disallowed-positions*.rs: ditto. - `tests/ui/proc-macro/macro-rules-derive-cfg.rs`: the diff looks large but the only difference is the insertion of a single invisible-delimited group around a metavar.
`NtBlock` is the last remaining variant of `Nonterminal`, so once it is gone then `Nonterminal` can be removed as well.
It's no longer needed. This does slightly worsen the error message for a single test, but that test contains code that is so badly broken that I'm not worried about it.
…e bound lifetimes
|
r? @davidtwco rustbot has assigned @davidtwco. Use |
rustbot
added
S-waiting-on-review
|
Some changes occurred in src/tools/rustfmt cc @rust-lang/rustfmt HIR ty lowering was modified cc @fmease |
|
Ugh. Draft mode would've been nice. Sorry for the pings! |
|
@bors try |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Sep 14, 2024
[CRATER] Crater rollup This is a crater rollup of: * rust-lang#124141 * rust-lang#130285 * rust-lang#130367 **What is a crater rollup?** It's simply a (manually set-up) crater job that is run on all of the containing PRs together, and then we can set the crates list for each of these jobs to *just* the list of failures after it's done. It should cut out on the bulk of "normal" crates that do nothing and simply just waste time to build without being affected by the union of all of these changes. After this is finished, I will adjust all of the jobs to use only the list of failed crates. That should significantly speed up these jobs from taking like ~6 days to taking ~2. See the last time I did this: rust-lang#129660. Given that rust-lang#130285 is running in build-and-test mode, let's run all of them in build-and-test mode.
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
🤦 rebased incorrectly! |
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
💔 Test failed - checks-actions |
bors
added
S-waiting-on-author
|
Ugh actually I don't wanna deal with this. Crater be damned! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a crater rollup of:
NonterminalandTokenKind::Interpolated#124141What is a crater rollup? It's simply a (manually set-up) crater job that is run on all of the containing PRs together, and then we can set the crates list for each of these jobs to just the list of failures after it's done. It should cut out on the bulk of "normal" crates that do nothing and simply just waste time to build without being affected by the union of all of these changes.
After this is finished, I will adjust all of the jobs to use only the list of failed crates. That should significantly speed up these jobs from taking like ~6 days to taking ~2. See the last time I did this: #129660.
Given that #130285 is running in build-and-test mode, let's run all of them in build-and-test mode.