Less maybe_whole_expr, take 2

[nnethercote]

I first tried this in #107550. I now think it's worth doing again, as a precursor to #124141.

r? @petrochenkov

[petrochenkov]

This needs a crater run because the error is now reported even under #[cfg(FALSE)].
@bors try

[bors]

⌛ Trying commit 74a18a0 with merge 397f895...

[bors]

☀️ Try build successful - checks-actions
Build commit: 397f895 (397f8958d101b259f6baaf92d107d770c95c3ce0)

[petrochenkov]

@craterbot check

[craterbot]

👌 Experiment pr-126571 created and queued.
🤖 Automatically detected try build 397f895
🔍 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

[craterbot]

🚧 Experiment pr-126571 is now running

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

[craterbot]

🎉 Experiment pr-126571 is completed!
📊 927 regressed and 7 fixed (473144 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

[nnethercote]

Lots of crater failures. A quick scan indicates that the unic-ucd-segment crate is responsible for most of the failures. I checked and found that it builds ok with the first commit applied, but fails with the second commit. It has some complicated multi-level declarative macros. If I change parse_literal_maybe_minus to also handle ExprKind::MacCall (in the same way it handles ExprKind::Lit) then unic-ucd-segment compiles again.

[nnethercote]

rustsec also fails, and it needs ExprKind::Path to be handled in parse_literal_maybe_minus.

It's interesting that can_begin_literal_maybe_minus does not include ExprKind::{MacCall,Path}.

[petrochenkov]

In the token based model neither mac!() nor path::to::somewhere are any close to literals, so I don't expect this to work even if the none-delimited groups become "untyped" some time after the removal of nonterminals.
So these cases should probably report a deprecation lint.

[nnethercote]

The ExprKind::MacCall case for unic-ucd-segment looks like stringify!(CR).

The ExprKind::Path case in rustsec is just an identifier YEAR_MIN, not a full path.

I will update the code to allow these two ExprKinds, and then do another crater run on the failing crates, to see if there are any other ExprKinds occurring.

[nnethercote]

Hmm, looking at the ExprKind::Path case some more, I think the real problem is in rustc. Consider this code:

macro_rules! range {
    ($min:expr, $max:expr) => {
        $min ..= $max
    }
}

const MAX: u32 = 10;

fn main() {
    match 3 {
        range!(0, MAX) => {}
        _ => {}
    }
}

It compiles fine with stable rustc. With the original version of this PR applied (i.e. without ExprKind::Path handling added to parse_maybe_literal_minus) it fails like this:

error: unexpected token: expression `MAX`
  --> p.rs:3:18
   |
3  |         $min ..= $max
   |                  ^^^^
...
11 |         range!(0, MAX) => {}
   |         --------------
   |         |
   |         this macro call doesn't expand to a pattern
   |         in this macro invocation
   |
   = note: this error originates in the macro `range` (in Nightly builds, run with -Z macro-backtrace for more info)

Adding ExprKind::Path handling to parse_maybe_literal_minus makes it compile again, but that's not the right solution. The real problem is in parse_pat_range_end:

rust/compiler/rustc_parse/src/parser/pat.rs

Lines 960 to 976 in 4333551

	let bound = if self.check_inline_const(0) {
	self.parse_const_block(self.token.span, true)
	} else if self.check_path() {
	let lo = self.token.span;
	let (qself, path) = if self.eat_lt() {
	// Parse a qualified path
	let (qself, path) = self.parse_qpath(PathStyle::Pat)?;
	(Some(qself), path)
	} else {
	// Parse an unqualified path
	(None, self.parse_path(PathStyle::Pat)?)
	};
	let hi = self.prev_token.span;
	Ok(self.mk_expr(lo.to(hi), ExprKind::Path(qself, path)))
	} else {
	self.parse_literal_maybe_minus()
	}?;

What happens in check_path? It fails! Because it relies on Token::is_path_start, which relies on Token::is_whole_path, which succeeds for NtPath but not for NtExpr(... ExprKind::Path ...).

So then we fall through to parse_maybe_literal_minus, which succeeds on stable because it accepts any NtExpr, but fails with this PR because it doesn't accept NtExpr(... ExprKind::Path ...).

The real solution is that parse_pat_range_end should handle NtExpr(... ExprKind::Path ...) before calling parse_maybe_literal_minus. There's almost certainly a similar problem for the start of pattern ranges.

I don't know yet if the MacCall case is like this or not.

[nnethercote]

While looking at the ExprKind::Path stuff more closely, I found this in parse_path_inner:

rust/compiler/rustc_parse/src/parser/path.rs

Lines 196 to 206 in 684b355

	maybe_whole!(self, NtPath, |path| reject_generics_if_mod_style(self, path.into_inner()));
	if let token::Interpolated(nt) = &self.token.kind {
	if let token::NtTy(ty) = &**nt {
	if let ast::TyKind::Path(None, path) = &ty.kind {
	let path = path.clone();
	self.bump();
	return Ok(reject_generics_if_mod_style(self, path));
	}
	}
	}

It accepts NtPath and NtTy(... TyKind::Path ...). @dtolnay added the latter part in #91150. I think it will need to allow NtExpr(... ExprKind::Path ...) to address the problem I described in the previous comment. And then I wonder if NtPat(... PatKind::Path ...) should also be allowed.

[nnethercote]

@bors try

[bors]

⌛ Trying commit 379b761 with merge 0ecd73a...

[bors]

☀️ Try build successful - checks-actions
Build commit: 0ecd73a (0ecd73ac69e42f0c7915911f31169e93143ae55d)

[nnethercote]

Getting the path stuff right is a bit tricky, so I think it's worth merging just the first commit here. Let's re-do the failures from the last crater run:

@craterbot check p=1 crates=https://crater-reports.s3.amazonaws.com/pr-126571/retry-regressed-list.txt

[craterbot]

👌 Experiment pr-126571-1 created and queued.
🤖 Automatically detected try build 0ecd73a
🔍 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

[craterbot]

🚧 Experiment pr-126571-1 is now running

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

[craterbot]

🎉 Experiment pr-126571-1 is completed!
📊 0 regressed and 0 fixed (927 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

[nnethercote]

No failures when re-running the crater failures with just the first commit applied. I think it's worth merging now and thinking about the additional restrictions on ExprKind in a follow-up. @petrochenkov, what do you think?

[petrochenkov]

@bors r+

[bors]

📌 Commit 379b761 has been approved by petrochenkov

It is now in the queue for this repository.

[nnethercote]

@bors rollup