matches! problem with edition 2021

[leonardo-m]

If I compile this:

fn main() {
    let _b = matches!(b'3', b'0' ..= b'9');
}

With:

rustc 1.53.0-nightly (b84932674 2021-04-21)
binary: rustc
commit-hash: b849326744a8eec939e592f0ab13bff85cc865d3
commit-date: 2021-04-21
host: x86_64-pc-windows-gnu
release: 1.53.0-nightly
LLVM version: 12.0.0

If I use the command:

...>rustc --edition 2018 -Z unstable-options test.rs

The compilation seems OK. If I compile with:

...>rustc --edition 2021 -Z unstable-options test.rs

rustc gives an error without line numbers:

error: `$pattern:pat` may be followed by `|`, which is not allowed for `pat` fragments
    |
    = note: allowed there are: `=>`, `,`, `=`, `if` or `in`

error: aborting due to previous error

[ehuss]

I believe this was caused by #83468 cc @hi-rustin

There seems to be some edition hygiene problem here. The matches macro in std should be 2018, and thus use the pat2015 behavior.

This makes it impossible to test 2021 on any project that uses the matches! macro.

[Rustin170506]

There seems to be some edition hygiene problem here. The matches macro in std should be 2018, and thus use the pat2015 behavior.

Yes, The pat2021 matcher will match new patterns, which include the | character. @nikomatsakis How do you think this can be fixed? Do we need to adjust matches! Or is there another way?

[ghost]

Do we need to adjust matches!

I suppose this should be fixed in the compiler by correcting the edition hygiene? I don't think it should be fixed by adjusting matches!, otherwise all macros from pre-2021-edition crates that use pat in this way will also need to be adjusted to be used in a 2021-edition crate.

@rustbot claim

[nikomatsakis]

Interesting! Let me poke a second into this.

[nikomatsakis]

@hyd-dev do you need any help or do you think you know what to do? Your diagnosis sounds correct to me.

[ghost]

Thanks @nikomatsakis! I don't know what to do yet and am digging into it now.

[Aaron1011]

This is caused by the fact that we currently map ExpnId::root() from a foreign crate to ExpnId::root() frrom the current crate during hygiene decoding. However the two root ExpnIds may have a different edition, so we end up using the current crate's edition instead of the foreign crate's edition.

[apiraino]

Assigning priority as discussed as part of the Prioritization Working Group procedure and removing I-prioritize.

@rustbot label -I-prioritize +P-medium

[camelid]

Increasing to P-high as discussed in the prioritization working group.

[Aaron1011]

I made an initial attempt at changing the ExpnId 'tree' into a forest - that is, we have multiple independent root ExpnData, one for each crate. However, this breaks a large amount of code that uses ExpnId::is_descendant_of. Previously, you could walk up the tree and be guaranteed to eventually have this method return true (when you reached the root at the latest). Now, you can two ExpnIds from disjoint trees, so they will never be related to each other, even if you walk to the root of the tree.

I don't really understand how the ExpnId.parent tree works, or how it interacts with resolution, so I'm not really sure what behavior we want here. A simpler approach might be to attach the root ExpnIds from other crates to our current root - this would give us a tree like

[crate_root_expn #0]  -> [non_root #1] -> [non_imported_expn #2]
                      -> [other_crate_root #3] -> [imported_expn #4]

This would preserve the properties relied by callers of ExpnId::is_descendant_of. However, I have no idea if this is the behavior we want.

cc @petrochenkov

[nikomatsakis]

@Aaron1011 have you had any more time to look at this? I'm trying to decide what it's going to take to get this fixed by end of week.

[Aaron1011]

I've started a discussion on Zulip (https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/Cross.20crate-hygiene.20roots) - there are some unresolved questions about how we want the general solution ('correct' cross-crate decoding of ExpnId::root() and SyntaxContext::root()) to work. However, it should be possible to make a hack that just fixes the problems for macros: when we're checking the span of a token for a foreign macro, fall back to the edition of that foreign crate if the SyntaxContext is SytaxContext::root().

[Aaron1011]

I've opened #85709 with a macro_rules!-specific fix, allowing us to avoid the larger problem of cross-crate hygiene (at least for the time being).

[nikomatsakis]

@Aaron1011 👍