Auto merge of rust-lang#13220 - y21:issue13219, r=dswij

[`macro_metavars_in_unsafe`]: recognize metavariables in tail expressions

Fixes rust-lang#13219

`macro_metavars_in_unsafe` keeps track of the current "expansion depth" (incremented/decremented when entering/leaving a macro span) to tell if an expression from the root context is contained within a macro (see the doc comment I added for a hopefully better explanation)

Before this PR, we didn't increment said `expn_depth` for `unsafe` blocks within macros, because we already do that in `visit_stmt` anyway, so it would work fine for statements, but that's not enough for tail expressions of an unsafe block.

So we now also increment it for macro unsafe blocks.
Also updated the comment for `expn_depth` while I'm at it because "This is not necessary for correctness" isn't correct now that I think about it

------

changelog: none

Author: bors

clippy_lints/src/macro_metavars_in_unsafe.rs

@@ -122,8 +122,23 @@ struct BodyVisitor<'a, 'tcx> {
     /// within a relevant macro.
     macro_unsafe_blocks: Vec<HirId>,
     /// When this is >0, it means that the node currently being visited is "within" a
-    /// macro definition. This is not necessary for correctness, it merely helps reduce the number
-    /// of spans we need to insert into the map, since only spans from macros are relevant.
+    /// macro definition.
+    /// This is used to detect if an expression represents a metavariable.
+    ///
+    /// For example, the following pre-expansion code that we want to lint
+    /// ```ignore
+    /// macro_rules! m { ($e:expr) => { unsafe { $e; } } }
+    /// m!(1);
+    /// ```
+    /// would look like this post-expansion code:
+    /// ```ignore
+    /// unsafe { /* macro */
+    ///     1 /* root */; /* macro */
+    /// }
+    /// ```
+    /// Visiting the block and the statement will increment the `expn_depth` so that it is >0,
+    /// and visiting the expression with a root context while `expn_depth > 0` tells us
+    /// that it must be a metavariable.
     expn_depth: u32,
     cx: &'a LateContext<'tcx>,
     lint: &'a mut ExprMetavarsInUnsafe,
@@ -157,7 +172,9 @@ impl<'a, 'tcx> Visitor<'tcx> for BodyVisitor<'a, 'tcx> {
             && (self.lint.warn_unsafe_macro_metavars_in_private_macros || is_public_macro(self.cx, macro_def_id))
         {
             self.macro_unsafe_blocks.push(block.hir_id);
+            self.expn_depth += 1;
             walk_block(self, block);
+            self.expn_depth -= 1;
             self.macro_unsafe_blocks.pop();
         } else if ctxt.is_root() && self.expn_depth > 0 {
             let unsafe_block = self.macro_unsafe_blocks.last().copied();

tests/ui-toml/macro_metavars_in_unsafe/default/test.rs

@@ -1,7 +1,7 @@
 //! Tests macro_metavars_in_unsafe with default configuration
 #![feature(decl_macro)]
 #![warn(clippy::macro_metavars_in_unsafe)]
-#![allow(clippy::no_effect)]
+#![allow(clippy::no_effect, clippy::not_unsafe_ptr_arg_deref)]
 
 #[macro_export]
 macro_rules! allow_works {
@@ -237,6 +237,19 @@ macro_rules! nested_macros {
     }};
 }
 
+pub mod issue13219 {
+    #[macro_export]
+    macro_rules! m {
+        ($e:expr) => {
+            // Metavariable in a block tail expression
+            unsafe { $e }
+        };
+    }
+    pub fn f(p: *const i32) -> i32 {
+        m!(*p)
+    }
+}
+
 fn main() {
     allow_works!(1);
     simple!(1);

tests/ui-toml/macro_metavars_in_unsafe/default/test.stderr

@@ -1,3 +1,15 @@
+error: this macro expands metavariables in an unsafe block
+  --> tests/ui-toml/macro_metavars_in_unsafe/default/test.rs:245:13
+   |
+LL |             unsafe { $e }
+   |             ^^^^^^^^^^^^^
+   |
+   = note: this allows the user of the macro to write unsafe code outside of an unsafe block
+   = help: consider expanding any metavariables outside of this block, e.g. by storing them in a variable
+   = help: ... or also expand referenced metavariables in a safe context to require an unsafe block at callsite
+   = note: `-D clippy::macro-metavars-in-unsafe` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::macro_metavars_in_unsafe)]`
+
 error: this macro expands metavariables in an unsafe block
   --> tests/ui-toml/macro_metavars_in_unsafe/default/test.rs:19:9
    |
@@ -10,8 +22,6 @@ LL | |         }
    = note: this allows the user of the macro to write unsafe code outside of an unsafe block
    = help: consider expanding any metavariables outside of this block, e.g. by storing them in a variable
    = help: ... or also expand referenced metavariables in a safe context to require an unsafe block at callsite
-   = note: `-D clippy::macro-metavars-in-unsafe` implied by `-D warnings`
-   = help: to override `-D warnings` add `#[allow(clippy::macro_metavars_in_unsafe)]`
 
 error: this macro expands metavariables in an unsafe block
   --> tests/ui-toml/macro_metavars_in_unsafe/default/test.rs:30:9
@@ -183,5 +193,5 @@ LL | |         }
    = help: consider expanding any metavariables outside of this block, e.g. by storing them in a variable
    = help: ... or also expand referenced metavariables in a safe context to require an unsafe block at callsite
 
-error: aborting due to 14 previous errors
+error: aborting due to 15 previous errors