Fix edition for pat fragments

[ghost]

The edition span.edition() returned seems wrong (at least it's Edition2021 in the example of #84429) -- the correct one seems to be provided by the edition parameter of rustc_expand::mbe::macro_rules::compile_declarative_macro().

Fixes #84429.

[rust-highfive]

r? @petrochenkov

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

[petrochenkov]

The current behavior is correct, matches! is defined by a 2018 edition crate, so it uses 2018 edition rules through edition hygiene.
If different behavior is desired, then matches! should use pat2021 explicitly.

[ghost]

@petrochenkov The current behavior is that matches! is using 2021 edition rules when used in a 2021 edition crate.

[petrochenkov]

After a more detailed look I see what is the issue.
For some reason edition of the pat identifier (which guides the follow token check) turns out to be 2021, while it should be 2018 (because matches is defined in a 2018 edition crate, and the pat is written in that crate).

So the correct edition is lost somewhere on the way from decoding (or even encoding) the macro to parsing its left hand side as tokens.
We need to figure out where exactly it's lost, and fix that place.

This PR replaces a per-token edition check with a global edition check, which happens to work in this specific case, but isn't really correct.

[petrochenkov]

Tokens decoded by CrateMetadataRef::get_macro already have incorrect editions.

UPD: Before being encoded in encode_info_for_macro_def the tokens have correct editions.
cc @Aaron1011

UPD: I don't have time to investigate this further today.

[ghost]

Tokens decoded by CrateMetadataRef::get_macro already have incorrect editions.

Yes, good point, I just found that too. Thanks.

which happens to work in this specific case, but isn't really correct.

Does that mean pat of a macro defined in another macro that is in a different edition (if possible at all) will be broken? If so, I think it would be good to add a test for that.

[petrochenkov]

Does that mean pat of a macro defined in another macro that is in a different edition (if possible at all) will be broken?

Yes, something like

// One edition
macro pass(item:$item) { $item }

// Another edition
pass! {
    macro matches(... $pat: pat | ...) { ... }
}

should work as a test.

[Aaron1011]

It turns out that we've been decoding editions incorrectly for root SyntaxContexts this entire time:

rust/compiler/rustc_span/src/hygiene.rs

Lines 1060 to 1065 in 58bdb08

	// Do this after decoding, so that we decode a `CrateNum`
	// if necessary
	if index == ExpnId::root().as_u32() {
	debug!("decode_expn_id: deserialized root");
	return Ok(ExpnId::root());
	}

This will decode an ExpnId::root() from a foreign crate as an ExpnId::root() from the current crate, which means that we'll use the edition from the current crate. When I wrote this code, I forgot that ExpnId::root() had data attached to it, so I thought it would be fine to map all ExpnId::root()s together.

I'll work on a fix.

[Aaron1011]

One tricky aspect to this is that there are many places where we compare ExpnIds for equality (either directly, or indirectly through a Span). This means that two SyntaxContexts from different crates will no longer be considered equal, even if those crates have the same edition (unless we modify decoding based on whether or not editions match, which seems like a bad idea).

I'm not sure if this will create any problems, especially around some of the macros-2.0 logic.

[petrochenkov]

Ah, this is the reason.
When reviewing the hygiene encoding/decoding PR I kind of expected the mapping of all roots into the same ExpnId to be a temporary solution. In theory they should have separate ExpnIds, and this issue gives one practical reason as well.

Some code working on ExpnId hierarchies may need updating, especially if it's doing direct comparisons with ExpnId::root() instead of using methods like ExpnData::is_root.

[Aaron1011]

@petrochenkov: So, the idea would be that we have multiple 'roots':

[current_crate root #0]     [foreign crate root #2]
		|							|
		|							|
[other_ctxt #1]				[other_ctxt #3]

[petrochenkov]

Yes.
I'm just not sure exactly how much code needs to be changed to make this work.

[petrochenkov]

If matches! needs to be fixed soon (which I suppose is the case), then it still should be able to use an explicitly disambiguated matcher like pat2015 or pat2021.

[ehuss]

If matches! needs to be fixed soon (which I suppose is the case), then it still should be able to use an explicitly disambiguated matcher like pat2015 or pat2021.

I personally don't think it is high enough priority to add some kind of exception right away. The only people testing 2021 are compiler developers, and I don't think this is necessarily blocking anyone right this moment. If this can be fixed within a few weeks, I think that should be sufficient timing-wise. Testing was supposed to start mid-may, but the schedule has slipped quite a bit.

Also, this affects anyone doing a similar style (for example, this macro in cssparser). Although matches! is probably the most popular macro with this idiom by far, full-scale testing will still be blocked by a proper fix.

[ghost]

I think I can close this PR now?

[nikomatsakis]

Is there an issue to track the fixup work here?

[nikomatsakis]

I guess it is #84429