Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

migration lint for expr2024 for the edition 2024 #125627

Merged
merged 2 commits into from
Jul 10, 2024

Conversation

vincenzopalazzo
Copy link
Member

This is adding a migration lint for the current (in the 2021 edition and previous)
to move expr to expr_2021 from expr

Issue #123742

I created also a repository to test out the migration https://github.com/vincenzopalazzo/expr2024-cargo-fix-migration

Co-Developed-by: @eholk

@rustbot
Copy link
Collaborator

rustbot commented May 27, 2024

r? @michaelwoerister

rustbot has assigned @michaelwoerister.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot 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 May 27, 2024
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@vincenzopalazzo
Copy link
Member Author

r? @traviscross

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@traviscross
Copy link
Contributor

r? @eholk

@rustbot rustbot assigned eholk and unassigned traviscross May 28, 2024
@rust-log-analyzer

This comment has been minimized.

@vincenzopalazzo vincenzopalazzo force-pushed the macros/cargo-fix-expr2024 branch 4 times, most recently from d11c856 to 664aefe Compare May 31, 2024 16:26
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

Copy link
Contributor

@eholk eholk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be in a pretty decent shape, but I left a few comments.

@rust-log-analyzer

This comment has been minimized.

Comment on lines 42 to 73
/// ### Explanation
///
/// Rust [editions] allow the language to evolve without breaking
/// backwards compatibility. This lint catches code that uses new keywords
/// that are added to the language that are used as identifiers (such as a
/// variable name, function name, etc.). If you switch the compiler to a
/// new edition without updating the code, then it will fail to compile if
/// you are using a new keyword as an identifier.
///
/// This lint solves the problem automatically. It is "allow" by default
/// because the code is perfectly valid in older editions. The [`cargo
/// fix`] tool with the `--edition` flag will switch this lint to "warn"
/// and automatically apply the suggested fix from the compiler.
/// This provides a completely automated way to update old code for
/// a new edition.
///
/// [editions]: https://doc.rust-lang.org/edition-guide/
/// [`cargo fix`]: https://doc.rust-lang.org/cargo/commands/cargo-fix.html
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs further changes, e.g. to explain what this lint does and what people should know about it (e.g. that they probably want to switch back to expr from expr_2021), and to remove text from the explanation that is not correct for what this lint does.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@traviscross we update the docs with @eholk so should be good for another review

@eholk
Copy link
Contributor

eholk commented Jul 8, 2024

This incorrectly suggests changing the literal identifier expr in macros:

macro_rules! test {
    (expr) => {}
}

test!(expr);

which suggests:

help: to keep the existing behavior, use the `expr_2021` fragment specifier.
  |
LL |     (expr_2021) => {}
  |      ~~~~~~~~~

It'd be good to add a test case to make sure we don't do this.

Ah, it looks like such a tests exists: https://github.com/rust-lang/rust/pull/125627/files#diff-af322a90f8768b5f39a55aa9cbdce92b80b502f4e8b5d84c0959e93d73ddc7d4

Carry on.

@rust-log-analyzer

This comment has been minimized.

@vincenzopalazzo vincenzopalazzo force-pushed the macros/cargo-fix-expr2024 branch 2 times, most recently from 1ee9ad2 to 1827af9 Compare July 9, 2024 17:40
This is adding a migration lint for the current (in the 2021 edition and previous)
to move expr to expr_2021 from expr

Co-Developed-by: Eric Holk
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Co-Developed-by: Eric Holk
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
@vincenzopalazzo
Copy link
Member Author

@compiler-errors When you have time I would like to have another review on the parsing code to see if the code can have your bless 😸

@compiler-errors
Copy link
Member

This looks fine to me for now. Please rebase to squash that fixup! commit, since bors doesn't autosquash.

@vincenzopalazzo
Copy link
Member Author

This looks fine to me for now. Please rebase to squash that fixup! commit, since bors doesn't autosquash.

Concurrency problem, I did while you was writing the comment sorry 😸

@compiler-errors
Copy link
Member

@bors r=compiler-errors,eholk rollup

@bors
Copy link
Contributor

bors commented Jul 9, 2024

📌 Commit 568e78f has been approved by compiler-errors,eholk

It is now in the queue for this repository.

@bors 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 9, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 9, 2024
…xpr2024, r=compiler-errors,eholk

migration lint for `expr2024` for the edition 2024

This is adding a migration lint for the current (in the 2021 edition and previous)
to move expr to expr_2021 from expr

Issue rust-lang#123742

I created also a repository to test out the migration https://github.com/vincenzopalazzo/expr2024-cargo-fix-migration

Co-Developed-by: `@eholk`
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 9, 2024
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#124339 (allow overwriting the output of `rustc --version`)
 - rust-lang#125627 (migration lint for `expr2024` for the edition 2024)
 - rust-lang#127091 (impl FusedIterator and a size hint for the error sources iter)
 - rust-lang#127358 (Automatically taint when reporting errors from ItemCtxt)
 - rust-lang#127484 (`#[doc(alias)]`'s doc: say that ASCII spaces are allowed)
 - rust-lang#127495 (More trait error reworking)
 - rust-lang#127496 (Update `f16`/`f128` FIXMEs that needed `(NEG_)INFINITY`)
 - rust-lang#127508 (small search graph refactor)
 - rust-lang#127521 (Remove spastorino from SMIR)
 - rust-lang#127532 (documentation: update cmake version)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 10, 2024
…xpr2024, r=compiler-errors,eholk

migration lint for `expr2024` for the edition 2024

This is adding a migration lint for the current (in the 2021 edition and previous)
to move expr to expr_2021 from expr

Issue rust-lang#123742

I created also a repository to test out the migration https://github.com/vincenzopalazzo/expr2024-cargo-fix-migration

Co-Developed-by: ```@eholk```
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 10, 2024
Rollup of 8 pull requests

Successful merges:

 - rust-lang#124211 (Bump `elided_lifetimes_in_associated_constant` to deny)
 - rust-lang#125627 (migration lint for `expr2024` for the edition 2024)
 - rust-lang#127091 (impl FusedIterator and a size hint for the error sources iter)
 - rust-lang#127461 (Fixup failing fuchsia tests)
 - rust-lang#127484 (`#[doc(alias)]`'s doc: say that ASCII spaces are allowed)
 - rust-lang#127508 (small search graph refactor)
 - rust-lang#127521 (Remove spastorino from SMIR)
 - rust-lang#127532 (documentation: update cmake version)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 21a0e86 into rust-lang:master Jul 10, 2024
6 checks passed
@rustbot rustbot added this to the 1.81.0 milestone Jul 10, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jul 10, 2024
Rollup merge of rust-lang#125627 - vincenzopalazzo:macros/cargo-fix-expr2024, r=compiler-errors,eholk

migration lint for `expr2024` for the edition 2024

This is adding a migration lint for the current (in the 2021 edition and previous)
to move expr to expr_2021 from expr

Issue rust-lang#123742

I created also a repository to test out the migration https://github.com/vincenzopalazzo/expr2024-cargo-fix-migration

Co-Developed-by: ``@eholk``
@vincenzopalazzo vincenzopalazzo deleted the macros/cargo-fix-expr2024 branch July 10, 2024 10:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants