recognize metavariables in tail expressions

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
+    /// macro definition.
-    /// of spans we need to insert into the map, since only spans from macros are relevant.
+    /// 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 @@ fn visit_expr(&mut self, e: &'tcx Expr<'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