Skip to content

keyword-idents is incompatible with macros #133709

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

Open
ehuss opened this issue Dec 1, 2024 · 1 comment
Open

keyword-idents is incompatible with macros #133709

ehuss opened this issue Dec 1, 2024 · 1 comment
Labels
A-edition-2024 Area: The 2024 edition A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` C-bug Category: This is a bug. D-edition Diagnostics: An error or lint that should account for edition differences. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. I-edition-triaged Issue: This issue has been reviewed and triaged by the Edition team. L-keyword_idents Lint group: keyword_idents L-keyword_idents_2024 Lint: keyword_idents_2024 T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@ehuss
Copy link
Contributor

ehuss commented Dec 1, 2024

The keyword_idents lint has incorrect suggestions in macro_rules macros (there are potentially separate concerns for the definition versus invocation). It suggests changing a new keyword to the raw identifier, but this is a semantic change (see #49520).

LHS Definition

Consider:

// edition 2021
#![warn(keyword_idents)]
#[macro_export]
macro_rules! m {
    (gen) => {};
}

This suggests changing it to a raw identifier:

@@ -1,5 +1,5 @@
 #![warn(keyword_idents)]
 #[macro_export]
 macro_rules! m {
-    (gen) => {};
+    (r#gen) => {};
 }

However, this is a semantic change because macro_rules does not treat an equivalence between raw and normal identifiers (or keywords). Thus, another crate using this macro:

// any edition
m!(gen);

will fail with the error "no rules expected gen", or "no rules expected reserved keyword gen" in 2024.

Invocation

Perhaps more difficult to resolve is the rewrites for an invocation.

Let's say we have a crate (any edition), with something like:

// crate a, any edition
#[macro_export]
macro_rules! other_macro {
    (gen) => {};
}

and then I am trying to migrate my crate to 2024:

// edition 2021
#![warn(keyword_idents)]
a::other_macro!(gen);

This will suggest to change it to a::other_macro!(r#gen), which is incorrect because that does not match any matchers.

I say this is more difficult, because there are some situations where changing the invocation is the correct thing to do. Consider:

// crate a, any edition
#[macro_export]
macro_rules! other_macro {
    ($e:stmt) => {};
}

and trying to migrate another crate to 2024:

// edition 2021
#![warn(keyword_idents)]
a::other_macro!(let gen = 1);

this gives the correct suggestion to rewrite it to a::other_macro!(let r#gen = 1);, which would fail to compile if it wasn't changed.

Possible solutions

There are different angles to tackle this problem:

  • Changing the suggestions provided by keyword_idents.
  • Changing the behavior of macros.

The latter seems like a bigger can of worms.

For the former, some rough thoughts:

  • Don't suggest changes on the LHS of a macro_rules matcher. This is clearly a semantics change.
  • Detect the situations where changing an invocation is incorrect, and avoid those suggestions. No idea how feasible that is.

Meta

rustc 1.85.0-nightly (7442931d4 2024-11-30)
binary: rustc
commit-hash: 7442931d49b199ad0a1cc0f8ca54e327b5139b66
commit-date: 2024-11-30
host: aarch64-apple-darwin
release: 1.85.0-nightly
LLVM version: 19.1.4
@ehuss ehuss added A-edition-2024 Area: The 2024 edition C-bug Category: This is a bug. D-edition Diagnostics: An error or lint that should account for edition differences. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. L-keyword_idents_2024 Lint: keyword_idents_2024 L-keyword_idents Lint group: keyword_idents labels Dec 1, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Dec 1, 2024
@jieyouxu jieyouxu added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Dec 2, 2024
@traviscross
Copy link
Contributor

@traviscross traviscross added the I-edition-triaged Issue: This issue has been reviewed and triaged by the Edition team. label Jan 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-edition-2024 Area: The 2024 edition A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` C-bug Category: This is a bug. D-edition Diagnostics: An error or lint that should account for edition differences. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. I-edition-triaged Issue: This issue has been reviewed and triaged by the Edition team. L-keyword_idents Lint group: keyword_idents L-keyword_idents_2024 Lint: keyword_idents_2024 T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants