-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Add tests for -Zunpretty=expanded
ported from stringify's tests
#125236
Conversation
8d9acab
to
bd9622d
Compare
} | ||
|
||
let paren_around_match; | ||
expr_as_stmt!(match paren_around_match {} | true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This provides coverage of what used to be tests/ui/macros/issue-98790.rs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe say it in comment with linking the issue address?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added link.
let no_paren = expr!(1 + 1) else { return; }; | ||
let paren_around_loop = expr!(loop {}) else { return; }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are from tests/ui/unpretty/let-else.rs
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very thorough! Thanks.
r=me once minor comments/questions are addressed/answered.
@@ -0,0 +1,99 @@ | |||
//@ compile-flags: -Zunpretty=expanded |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A brief comment at the top of this file explaining what it is testing would be helpful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added: "This test covers the AST pretty-printer's insertion of parentheses in some macro metavariable edge cases. Synthetic parentheses (i.e. not appearing in the syntax tree) need to be printed in order for the printed code to be valid Rust syntax. We also test negative cases: the pretty-printer should not be synthesizing parentheses indiscriminately; only where necessary."
} | ||
|
||
/// ExprKind::OffsetOf | ||
fn expr_offset_of() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why all the empty lines here? I don't understand this output.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sadly this is business-as-usual for the pretty printer. It has been broken like this for as long as I remember. The exact behavior is hard to characterize but it's something like: comments and blank lines are anchored to a byte offset in the file. In certain supported positions, the pretty printer will check the current byte offset in the output and flush all comments and blank lines that appeared at any lower byte offset in the input and has not already been printed. This is great if a syntax tree is parsed and immediately printed back out unmodified, because most comments can be preserved accurately in that case. But if we're printing output that is not identical to the input (such as because we performed macro-expansion) then the result is bogus.
It's also why I am using ///
throughout this file instead of //
. Attributes are anchored correctly to syntax tree nodes as opposed to byte locations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like in this case the issue is triggered by core::mem::offset_of!(T, field)
in the input. It expanded to { builtin # offset_of(T, field) }
in the output. But that {
token is "from a different file" because it's not from the original input file; and probably that "other file" has higher BytePos values than expanded-interpolation.rs. So every blank line and comment from the rest of expanded-interpolation.rs got flushed here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh, interesting. This would be useful information to put in a comment, also.
@bors r=nnethercote |
Rollup of 3 pull requests Successful merges: - rust-lang#125214 (Only make GAT ambiguous in `match_projection_projections` considering shallow resolvability) - rust-lang#125236 (Add tests for `-Zunpretty=expanded` ported from stringify's tests) - rust-lang#125251 (Clarify how String::leak and into_boxed_str differ ) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#125236 - dtolnay:expandtest, r=nnethercote Add tests for `-Zunpretty=expanded` ported from stringify's tests This PR adds a new set of tests for the AST pretty-printer. Previously, pretty-printer edge cases were tested by way of `stringify!` in [tests/ui/macros/stringify.rs](https://github.com/rust-lang/rust/blob/1.78.0/tests/ui/macros/stringify.rs), such as the tests added by rust-lang@419b269 and rust-lang@527e2ea. Those tests will no longer provide effective coverage of the AST pretty-printer after rust-lang#124141. `Nonterminal` and `TokenKind::Interpolated` are being removed, and a consequence is that `stringify!` will perform token stream pretty printing, instead of AST pretty printing, in all of the `stringify!` cases including $:expr and all other interpolations. This PR adds 2 new ui tests with `compile-flags: -Zunpretty=expanded`: - **tests/ui/unpretty/expanded-exhaustive.rs** — this test aims for exhaustive coverage of all the variants of `ExprKind`, `ItemKind`, `PatKind`, `StmtKind`, `TyKind`, and `VisibilityKind`. Some parts could use being fleshed out further, but the current state is roughly on par with what exists in the old stringify-based tests. - **tests/ui/unpretty/expanded-interpolation.rs** — this test covers tricky macro metavariable edge cases that require the AST pretty printer to synthesize parentheses in order for the printed code to be valid Rust syntax. r? `@nnethercote`
Instead of using AST pretty printing. This is a step towards removing `token::Interpolated`, which will eventually (in rust-lang#124141) be replaced with a token stream within invisible delimiters. This changes (improves) the output of the `stringify!` macro in some cases. This is allowed. As the `stringify!` docs say: "Note that the expanded results of the input tokens may change in the future. You should be careful if you rely on the output." Test changes: - tests/ui/macros/stringify.rs: this used to test both token stream pretty printing and AST pretty printing via different ways of invoking of `stringify!` (i.e. `$expr` vs `$tt`). But those two different invocations now give the same result, which is a nice consistency improvement. This removes the need for all the `c2*` macros. The AST pretty printer now has more thorough testing thanks to rust-lang#125236. - tests/ui/proc-macro/*: minor improvements where small differences between `INPUT (DISPLAY)` output and `DEEP-RE-COLLECTED (DISPLAY)` output disappear.
Instead of using AST pretty printing. This is a step towards removing `token::Interpolated`, which will eventually (in rust-lang#124141) be replaced with a token stream within invisible delimiters. This changes (improves) the output of the `stringify!` macro in some cases. This is allowed. As the `stringify!` docs say: "Note that the expanded results of the input tokens may change in the future. You should be careful if you rely on the output." Test changes: - tests/ui/macros/stringify.rs: this used to test both token stream pretty printing and AST pretty printing via different ways of invoking of `stringify!` (i.e. `$expr` vs `$tt`). But those two different invocations now give the same result, which is a nice consistency improvement. This removes the need for all the `c2*` macros. The AST pretty printer now has more thorough testing thanks to rust-lang#125236. - tests/ui/proc-macro/*: minor improvements where small differences between `INPUT (DISPLAY)` output and `DEEP-RE-COLLECTED (DISPLAY)` output disappear.
Instead of using AST pretty printing. This is a step towards removing `token::Interpolated`, which will eventually (in rust-lang#124141) be replaced with a token stream within invisible delimiters. This changes (improves) the output of the `stringify!` macro in some cases. This is allowed. As the `stringify!` docs say: "Note that the expanded results of the input tokens may change in the future. You should be careful if you rely on the output." Test changes: - tests/ui/macros/stringify.rs: this used to test both token stream pretty printing and AST pretty printing via different ways of invoking of `stringify!` (i.e. `$expr` vs `$tt`). But those two different invocations now give the same result, which is a nice consistency improvement. This removes the need for all the `c2*` macros. The AST pretty printer now has more thorough testing thanks to rust-lang#125236. - tests/ui/proc-macro/*: minor improvements where small differences between `INPUT (DISPLAY)` output and `DEEP-RE-COLLECTED (DISPLAY)` output disappear.
This PR adds a new set of tests for the AST pretty-printer.
Previously, pretty-printer edge cases were tested by way of
stringify!
in tests/ui/macros/stringify.rs, such as the tests added by 419b269 and 527e2ea.Those tests will no longer provide effective coverage of the AST pretty-printer after #124141.
Nonterminal
andTokenKind::Interpolated
are being removed, and a consequence is thatstringify!
will perform token stream pretty printing, instead of AST pretty printing, in all of thestringify!
cases including $:expr and all other interpolations.This PR adds 2 new ui tests with
compile-flags: -Zunpretty=expanded
:tests/ui/unpretty/expanded-exhaustive.rs — this test aims for exhaustive coverage of all the variants of
ExprKind
,ItemKind
,PatKind
,StmtKind
,TyKind
, andVisibilityKind
. Some parts could use being fleshed out further, but the current state is roughly on par with what exists in the old stringify-based tests.tests/ui/unpretty/expanded-interpolation.rs — this test covers tricky macro metavariable edge cases that require the AST pretty printer to synthesize parentheses in order for the printed code to be valid Rust syntax.
r? @nnethercote