Restrict `parse_maybe_literal_minus`
parse_literal_maybe_minus currently accepts to many kinds of NtExpr. This PR starts the process of restricting it.
r? @petrochenkov
The original version of #126571 tried to ban all NtExprs other than literals and unary minus, but a crater run
showed that ExprKind::Path and ExprKind::MacCall are still needed, which is why they are still allowed in this PR. To double-check this, we could re-run the failures from that first crater run, or even do a full crater run if we want to be extra careful.
There are follow-ups to be done to remove ExprKind::Path and Expr::MacCall from parse_literal_maybe_minus, but this PR is a sufficient first step.
@bors try
:hourglass: Trying commit 21efb726ba6f306121e1af18e2186e51d6c4d8d0 with merge 79097c2a1baf756f8e4297787c180a9db4e42301...
:sunny: Try build successful - checks-actions
Build commit: 79097c2a1baf756f8e4297787c180a9db4e42301 (79097c2a1baf756f8e4297787c180a9db4e42301)
@craterbot check p=1 crates=https://crater-reports.s3.amazonaws.com/pr-126571/retry-regressed-list.txt
:rotating_light: Error: missing desired crates: {"htmx-rs"}
:sos: If you have any trouble with Crater please ping @rust-lang/infra!
:information_source: Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more
@craterbot check p=1 crates=https://gist.githubusercontent.com/petrochenkov/d33e774c2065457db57d1dee3713f55a/raw/c43de2eb8109a15dce33173973a03ffc424e58b9/gistfile1.txt
:ok_hand: Experiment pr-129289 created and queued.
:robot: Automatically detected try build 79097c2a1baf756f8e4297787c180a9db4e42301
:mag: You can check out the queue and this experiment's details.
:information_source: Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more
:construction: Experiment pr-129289 is now running
:information_source: Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more
:tada: Experiment pr-129289 is completed!
:bar_chart: 1 regressed and 0 fixed (926 total)
:newspaper: Open the full report.
:warning: If you notice any spurious failure please add them to the blacklist! :information_source: Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more
There is one real regression. If I allow NtExpr(...ExprKind::Tup...) in parse_literal_maybe_minus that fixes the regression. I need to investigate some more to understand what's happening.
Here's an interesting test case:
macro_rules! p2 {
($m:pat) => {}
}
macro_rules! p1 {
($m:expr) => { p2!($m) };
}
fn main() {
// Reasonable expression patterns, that are currently accepted, and
// show up in a crater run.
p1!(());
p1!((3,4));
// Reasonable expression patterns, that are currently accepted, but
// dont't show up in a crater run.
p1!([3,4]);
p1!(&x);
p1!(..);
p1!(Foo(x, y));
p1!(Foo { x, y });
// Unreasonable expression patterns, that are currently accepted, but
// don't show up in a crater run.
p1!(f());
p1!(x.f());
p1!(1 + 1);
p1!(x as u32);
p1!(if a { b } else { c });
p1!(loop {});
p1!(x = 3);
p1!((3)); // maybe a reasonable pattern?
}
Currently this compiles without any problem because parse_literal_maybe_minus accepts any kind of NtExpr. With this PR's current code all of these lines would fail with syntax errors of the form expected pattern, found expression (). @petrochenkov, do you have opinions about which of these should be accepted?
do you have opinions about which of these should be accepted?
After migrating to the token model - everything that fits into both the expression and the pattern syntax as tokens.
That would be all the examples except x.f(), 1 + 1, x as u32, if a { b } else { c }, loop {} and x = 3.
Before the migration - no strong opinion, just not more than after the migration, and no large breakage.
Still waiting on the author to answer. @nnethercote Do you plan to return to this?
@nnethercote Do you plan to return to this?
Unsure. The current behaviour isn't great, but fixing it turns out to be more complex than I expected, so this PR has fallen a long way down my todo list :(