Make edition dependent `:expr` macro fragment act like the edition-dependent `:pat` fragment does #126700

compiler-errors · 2024-06-19T16:44:28Z

Parse the :expr fragment as :expr_2021 in editions <=2021, and as :expr in edition 2024. This is similar to how we parse :pat as :pat_param in edition <=2018 and :pat_with_or in >=2021, and means we can get rid of a span dependency from nonterminal_may_begin_with.

Specifically, this fixes a theoretical regression since the expr_2021 macro fragment previously would allow const {} if the caller is edition 2024. This is inconsistent with the way that the pat macro fragment was upgraded, and also leads to surprising behavior when a macro caller crate upgrades to edtion 2024, since they may have parsing changes that they never asked for (with no way of opting out of it).

This PR also allows using expr_2021 in all editions. Why was this was disallowed in the first place? It's purely additive, and also it's still feature gated?

r? @fmease @eholk cc @vincenzopalazzo
cc #123865

Tracking:

Tracking Issue for updating expr macro fragment specifier for Rust 2024 #123742

compiler-errors · 2024-06-19T17:00:50Z

compiler/rustc_ast/src/token.rs

@@ -884,7 +884,11 @@ pub enum NonterminalKind {
    PatWithOr,
    Expr,
    /// Matches an expression using the rules from edition 2021 and earlier.
-    Expr2021,
+    Expr2021 {


unrelated -- but I wonder if we can come up with a more descriptive name. expr_2021 gives me the vibe that its new to 2021 (i.e. >=2021) not legacy to 2021 (i.e. <=2021).

I quite like PatParam and PatWithOr naming scheme since it makes it less about edition and more about what it's parsing.

Yeah, that would be awesome. I couldn't come up with a good succinct name yet, it's kinda hard lol ngl and the grammar may still change, too (cc #126697). Note that we're already tracking this under Unresolved Questions in #123742.

I agree the _2021 suffix's meaning is non-obvious, and even after learning what it means it I keep having trouble remembering the meaning.

I have been thinking about a good name for a while now because I often get confused when reading the code a week later.

However, I think expr_2021 is one of the best names we can choose. If we update the documentation to explain that expr_2021 includes all features up to the 2021 edition, and expr includes all features of the current edition (up to 2024 in this case), it would be clear.

This approach can be applied retroactively as well. We do not have an expr_2018 because there were no changes in the semantics of expr at that time.

Do you think your concern can be addressed by updating the documentation to clearly explain this way of thinking?

I am also considering that if we give another name without referencing the edition, and if expr keeps changing in the next edition, keeping track of and explaining all the different expr types will be a mess, in my opinion.

Maybe we can use the year as a suffix for saying "the moment in was introduced", that would mean we would have Expr2015 and Expr2024; that would follow "the vibe that its new to 2024".

Just to confirm, are we agreeing that Expr2021 would include all features up to the 2021 edition, and Expr would continue to represent the current edition's features?

Or are you proposing a slightly different idea from what we are currently implementing?

By the way, I am happy with either approach, but I think it is important for users that we clearly indicate the edition where the expr_* is necessary to maintain old behavior.

fmease

Very good catch, that makes a lot of sense! Thanks :)

fmease · 2024-06-20T18:56:24Z

@bors r+

bors · 2024-06-20T18:56:26Z

📌 Commit 3e8898a has been approved by fmease

It is now in the queue for this repository.

Make edition dependent `:expr` macro fragment act like the edition-dependent `:pat` fragment does Parse the `:expr` fragment as `:expr_2021` in editions <=2021, and as `:expr` in edition 2024. This is similar to how we parse `:pat` as `:pat_param` in edition <=2018 and `:pat_with_or` in >=2021, and means we can get rid of a span dependency from `nonterminal_may_begin_with`. Specifically, this fixes a theoretical regression since the `expr_2021` macro fragment previously would allow `const {}` if the *caller* is edition 2024. This is inconsistent with the way that the `pat` macro fragment was upgraded, and also leads to surprising behavior when a macro *caller* crate upgrades to edtion 2024, since they may have parsing changes that they never asked for (with no way of opting out of it). This PR also allows using `expr_2021` in all editions. Why was this was disallowed in the first place? It's purely additive, and also it's still feature gated? r? `@fmease` `@eholk` cc `@vincenzopalazzo` cc rust-lang#123865 Tracking: - rust-lang#123742

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#125627 (migration lint for `expr2024` for the edition 2024) - rust-lang#126481 (Add `powerpc-unknown-openbsd` maintaince status) - rust-lang#126613 (Print the tested value in int_log tests) - rust-lang#126617 (Expand `avx512_target_feature` to include VEX variants) - rust-lang#126686 (Add `#[rustc_dump_{predicates,item_bounds}]`) - rust-lang#126700 (Make edition dependent `:expr` macro fragment act like the edition-dependent `:pat` fragment does) - rust-lang#126707 (Pass target to inaccessible-temp-dir rmake test) - rust-lang#126757 (Properly gate `safe` keyword in pre-expansion) r? `@ghost` `@rustbot` modify labels: rollup

Make edition dependent `:expr` macro fragment act like the edition-dependent `:pat` fragment does Parse the `:expr` fragment as `:expr_2021` in editions <=2021, and as `:expr` in edition 2024. This is similar to how we parse `:pat` as `:pat_param` in edition <=2018 and `:pat_with_or` in >=2021, and means we can get rid of a span dependency from `nonterminal_may_begin_with`. Specifically, this fixes a theoretical regression since the `expr_2021` macro fragment previously would allow `const {}` if the *caller* is edition 2024. This is inconsistent with the way that the `pat` macro fragment was upgraded, and also leads to surprising behavior when a macro *caller* crate upgrades to edtion 2024, since they may have parsing changes that they never asked for (with no way of opting out of it). This PR also allows using `expr_2021` in all editions. Why was this was disallowed in the first place? It's purely additive, and also it's still feature gated? r? ``@fmease`` ``@eholk`` cc ``@vincenzopalazzo`` cc rust-lang#123865 Tracking: - rust-lang#123742

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#124101 (Add PidFd::{kill, wait, try_wait}) - rust-lang#126125 (Improve conflict marker recovery) - rust-lang#126481 (Add `powerpc-unknown-openbsd` maintenance status) - rust-lang#126613 (Print the tested value in int_log tests) - rust-lang#126617 (Expand `avx512_target_feature` to include VEX variants) - rust-lang#126700 (Make edition dependent `:expr` macro fragment act like the edition-dependent `:pat` fragment does) - rust-lang#126707 (Pass target to inaccessible-temp-dir rmake test) - rust-lang#126767 (`StaticForeignItem` and `StaticItem` are the same) r? `@ghost` `@rustbot` modify labels: rollup

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#126125 (Improve conflict marker recovery) - rust-lang#126481 (Add `powerpc-unknown-openbsd` maintenance status) - rust-lang#126613 (Print the tested value in int_log tests) - rust-lang#126617 (Expand `avx512_target_feature` to include VEX variants) - rust-lang#126700 (Make edition dependent `:expr` macro fragment act like the edition-dependent `:pat` fragment does) - rust-lang#126707 (Pass target to inaccessible-temp-dir rmake test) - rust-lang#126767 (`StaticForeignItem` and `StaticItem` are the same) - rust-lang#126774 (Fix another assertion failure for some Expect diagnostics.) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#126700 - compiler-errors:fragment, r=fmease Make edition dependent `:expr` macro fragment act like the edition-dependent `:pat` fragment does Parse the `:expr` fragment as `:expr_2021` in editions <=2021, and as `:expr` in edition 2024. This is similar to how we parse `:pat` as `:pat_param` in edition <=2018 and `:pat_with_or` in >=2021, and means we can get rid of a span dependency from `nonterminal_may_begin_with`. Specifically, this fixes a theoretical regression since the `expr_2021` macro fragment previously would allow `const {}` if the *caller* is edition 2024. This is inconsistent with the way that the `pat` macro fragment was upgraded, and also leads to surprising behavior when a macro *caller* crate upgrades to edtion 2024, since they may have parsing changes that they never asked for (with no way of opting out of it). This PR also allows using `expr_2021` in all editions. Why was this was disallowed in the first place? It's purely additive, and also it's still feature gated? r? ```@fmease``` ```@eholk``` cc ```@vincenzopalazzo``` cc rust-lang#123865 Tracking: - rust-lang#123742

compiler-errors added 2 commits June 19, 2024 12:37

Add a test demonstrating the problem

50d1efa

Allow naming expr_2021 in all editions

3e8898a

rustbot assigned fmease Jun 19, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 19, 2024

compiler-errors mentioned this pull request Jun 19, 2024

[RFC] mbe: consider the _ in 2024 an expression #126697

Merged

compiler-errors commented Jun 19, 2024

View reviewed changes

fmease approved these changes Jun 20, 2024

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 20, 2024

fmease mentioned this pull request Jun 20, 2024

Tracking Issue for updating expr macro fragment specifier for Rust 2024 #123742

Closed

10 tasks

matthiaskrgr mentioned this pull request Jun 20, 2024

Rollup of 8 pull requests #126760

Closed

matthiaskrgr mentioned this pull request Jun 21, 2024

Rollup of 8 pull requests #126775

Closed

matthiaskrgr mentioned this pull request Jun 21, 2024

Rollup of 8 pull requests #126781

Merged

bors merged commit 3bd84f1 into rust-lang:master Jun 21, 2024
6 checks passed

rustbot added this to the 1.81.0 milestone Jun 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make edition dependent `:expr` macro fragment act like the edition-dependent `:pat` fragment does #126700

Make edition dependent `:expr` macro fragment act like the edition-dependent `:pat` fragment does #126700

compiler-errors commented Jun 19, 2024 •

edited by traviscross

Loading

compiler-errors Jun 19, 2024

fmease Jun 20, 2024

nnethercote Jun 21, 2024

vincenzopalazzo Jun 21, 2024

Urgau Jun 21, 2024

vincenzopalazzo Jun 21, 2024

fmease left a comment

fmease commented Jun 20, 2024

bors commented Jun 20, 2024

Make edition dependent :expr macro fragment act like the edition-dependent :pat fragment does #126700

Make edition dependent :expr macro fragment act like the edition-dependent :pat fragment does #126700

Conversation

compiler-errors commented Jun 19, 2024 • edited by traviscross Loading

compiler-errors Jun 19, 2024

Choose a reason for hiding this comment

fmease Jun 20, 2024

Choose a reason for hiding this comment

nnethercote Jun 21, 2024

Choose a reason for hiding this comment

vincenzopalazzo Jun 21, 2024

Choose a reason for hiding this comment

Urgau Jun 21, 2024

Choose a reason for hiding this comment

vincenzopalazzo Jun 21, 2024

Choose a reason for hiding this comment

fmease left a comment

Choose a reason for hiding this comment

fmease commented Jun 20, 2024

bors commented Jun 20, 2024

Make edition dependent `:expr` macro fragment act like the edition-dependent `:pat` fragment does #126700

Make edition dependent `:expr` macro fragment act like the edition-dependent `:pat` fragment does #126700

compiler-errors commented Jun 19, 2024 •

edited by traviscross

Loading