Auto merge of rust-lang#97903 - est31:unused_macro_rules_compile_error, r=petrochenkov

Never regard macro rules with compile_error! invocations as unused

The very point of compile_error! is to never be reached, and one of
the use cases of the macro, currently also listed as examples in the
documentation of compile_error, is to create nicer errors for wrong
macro invocations. Thus, we should never warn about unused macro arms
that contain invocations of compile_error.

See also rust-lang#96150 (comment) and the discussion after that.

Furthermore, the PR also contains two commits to silence `unused_macro_rules` when a macro has an invalid rule, and to add a test that `unused_macros` does not behave badly in the same situation.

r? `@petrochenkov` as I've talked to them about this

Author: bors

compiler/rustc_expand/src/lib.rs

@@ -1,4 +1,5 @@
 #![allow(rustc::potential_query_instability)]
+#![feature(array_windows)]
 #![feature(associated_type_bounds)]
 #![feature(associated_type_defaults)]
 #![feature(if_let_guard)]

compiler/rustc_expand/src/mbe/macro_rules.rs

@@ -380,7 +380,7 @@ pub fn compile_declarative_macro(
     features: &Features,
     def: &ast::Item,
     edition: Edition,
-) -> (SyntaxExtension, Vec<Span>) {
+) -> (SyntaxExtension, Vec<(usize, Span)>) {
     debug!("compile_declarative_macro: {:?}", def);
     let mk_syn_ext = |expander| {
         SyntaxExtension::new(
@@ -539,11 +539,22 @@ pub fn compile_declarative_macro(
         None => {}
     }
 
-    // Compute the spans of the macro rules
-    // We only take the span of the lhs here,
-    // so that the spans of created warnings are smaller.
-    let rule_spans = if def.id != DUMMY_NODE_ID {
-        lhses.iter().map(|lhs| lhs.span()).collect::<Vec<_>>()
+    // Compute the spans of the macro rules for unused rule linting.
+    // To avoid warning noise, only consider the rules of this
+    // macro for the lint, if all rules are valid.
+    // Also, we are only interested in non-foreign macros.
+    let rule_spans = if valid && def.id != DUMMY_NODE_ID {
+        lhses
+            .iter()
+            .zip(rhses.iter())
+            .enumerate()
+            // If the rhs contains an invocation like compile_error!,
+            // don't consider the rule for the unused rule lint.
+            .filter(|(_idx, (_lhs, rhs))| !has_compile_error_macro(rhs))
+            // We only take the span of the lhs here,
+            // so that the spans of created warnings are smaller.
+            .map(|(idx, (lhs, _rhs))| (idx, lhs.span()))
+            .collect::<Vec<_>>()
     } else {
         Vec::new()
     };
@@ -651,6 +662,29 @@ fn check_matcher(sess: &ParseSess, def: &ast::Item, matcher: &[mbe::TokenTree])
     err == sess.span_diagnostic.err_count()
 }
 
+fn has_compile_error_macro(rhs: &mbe::TokenTree) -> bool {
+    match rhs {
+        mbe::TokenTree::Delimited(_sp, d) => {
+            let has_compile_error = d.tts.array_windows::<3>().any(|[ident, bang, args]| {
+                if let mbe::TokenTree::Token(ident) = ident &&
+                        let TokenKind::Ident(ident, _) = ident.kind &&
+                        ident == sym::compile_error &&
+                        let mbe::TokenTree::Token(bang) = bang &&
+                        let TokenKind::Not = bang.kind &&
+                        let mbe::TokenTree::Delimited(_, del) = args &&
+                        del.delim != Delimiter::Invisible
+                    {
+                        true
+                    } else {
+                        false
+                    }
+            });
+            if has_compile_error { true } else { d.tts.iter().any(has_compile_error_macro) }
+        }
+        _ => false,
+    }
+}
+
 // `The FirstSets` for a matcher is a mapping from subsequences in the
 // matcher to the FIRST set for that subsequence.
 //

compiler/rustc_resolve/src/build_reduced_graph.rs

@@ -1220,12 +1220,12 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
         ident: Ident,
         def_id: LocalDefId,
         node_id: NodeId,
-        rule_spans: &[Span],
+        rule_spans: &[(usize, Span)],
     ) {
         if !ident.as_str().starts_with('_') {
             self.r.unused_macros.insert(def_id, (node_id, ident));
-            for (rule_i, rule_span) in rule_spans.iter().enumerate() {
-                self.r.unused_macro_rules.insert((def_id, rule_i), (ident, *rule_span));
+            for (rule_i, rule_span) in rule_spans.iter() {
+                self.r.unused_macro_rules.insert((def_id, *rule_i), (ident, *rule_span));
             }
         }
     }

compiler/rustc_resolve/src/macros.rs

@@ -870,7 +870,7 @@ impl<'a> Resolver<'a> {
         &mut self,
         item: &ast::Item,
         edition: Edition,
-    ) -> (SyntaxExtension, Vec<Span>) {
+    ) -> (SyntaxExtension, Vec<(usize, Span)>) {
         let (mut result, mut rule_spans) = compile_declarative_macro(
             &self.session,
             self.session.features_untracked(),

src/test/ui/lint/unused/unused-macro-rules-compile-error.rs

@@ -0,0 +1,27 @@
+#![deny(unused_macro_rules)]
+// To make sure we are not hitting this
+#![deny(unused_macros)]
+
+macro_rules! num {
+    (one) => { 1 };
+    // Most simple (and common) case
+    (two) => { compile_error!("foo"); };
+    // Some nested use
+    (two_) => { foo(compile_error!("foo")); };
+    (three) => { 3 };
+    (four) => { 4 }; //~ ERROR: rule of macro
+}
+const _NUM: u8 = num!(one) + num!(three);
+
+// compile_error not used as a macro invocation
+macro_rules! num2 {
+    (one) => { 1 };
+    // Only identifier present
+    (two) => { fn compile_error() {} }; //~ ERROR: rule of macro
+    // Only identifier and bang present
+    (two_) => { compile_error! }; //~ ERROR: rule of macro
+    (three) => { 3 };
+}
+const _NUM2: u8 = num2!(one) + num2!(three);
+
+fn main() {}

src/test/ui/lint/unused/unused-macro-rules-compile-error.stderr

@@ -0,0 +1,26 @@
+error: 5th rule of macro `num` is never used
+  --> $DIR/unused-macro-rules-compile-error.rs:12:5
+   |
+LL |     (four) => { 4 };
+   |     ^^^^^^
+   |
+note: the lint level is defined here
+  --> $DIR/unused-macro-rules-compile-error.rs:1:9
+   |
+LL | #![deny(unused_macro_rules)]
+   |         ^^^^^^^^^^^^^^^^^^
+
+error: 3rd rule of macro `num2` is never used
+  --> $DIR/unused-macro-rules-compile-error.rs:22:5
+   |
+LL |     (two_) => { compile_error! };
+   |     ^^^^^^
+
+error: 2nd rule of macro `num2` is never used
+  --> $DIR/unused-macro-rules-compile-error.rs:20:5
+   |
+LL |     (two) => { fn compile_error() {} };
+   |     ^^^^^
+
+error: aborting due to 3 previous errors
+

src/test/ui/lint/unused/unused-macro-rules-malformed-rule.rs

@@ -0,0 +1,11 @@
+#![deny(unused_macro_rules)]
+
+macro_rules! foo {
+    (v) => {};
+    (w) => {};
+    () => 0; //~ ERROR: macro rhs must be delimited
+}
+
+fn main() {
+    foo!(v);
+}

src/test/ui/lint/unused/unused-macro-rules-malformed-rule.stderr

@@ -0,0 +1,8 @@
+error: macro rhs must be delimited
+  --> $DIR/unused-macro-rules-malformed-rule.rs:6:11
+   |
+LL |     () => 0;
+   |           ^
+
+error: aborting due to previous error
+

src/test/ui/lint/unused/unused-macros-malformed-rule.rs

@@ -0,0 +1,15 @@
+#![deny(unused_macros)]
+
+macro_rules! foo { //~ ERROR: unused macro definition
+    (v) => {};
+    () => 0; //~ ERROR: macro rhs must be delimited
+}
+
+macro_rules! bar {
+    (v) => {};
+    () => 0; //~ ERROR: macro rhs must be delimited
+}
+
+fn main() {
+    bar!(v);
+}

src/test/ui/lint/unused/unused-macros-malformed-rule.stderr

@@ -0,0 +1,26 @@
+error: macro rhs must be delimited
+  --> $DIR/unused-macros-malformed-rule.rs:5:11
+   |
+LL |     () => 0;
+   |           ^
+
+error: macro rhs must be delimited
+  --> $DIR/unused-macros-malformed-rule.rs:10:11
+   |
+LL |     () => 0;
+   |           ^
+
+error: unused macro definition: `foo`
+  --> $DIR/unused-macros-malformed-rule.rs:3:14
+   |
+LL | macro_rules! foo {
+   |              ^^^
+   |
+note: the lint level is defined here
+  --> $DIR/unused-macros-malformed-rule.rs:1:9
+   |
+LL | #![deny(unused_macros)]
+   |         ^^^^^^^^^^^^^
+
+error: aborting due to 3 previous errors
+