From 234a1d35d9f1acc1da25b5069908ca2de209d322 Mon Sep 17 00:00:00 2001
From: y21 <30553356+y21@users.noreply.github.com>
Date: Mon, 5 Aug 2024 12:02:08 +0200
Subject: [PATCH] recognize metavariables in tail expressions

---
 clippy_lints/src/macro_metavars_in_unsafe.rs  | 21 +++++++++++++++++--
 .../macro_metavars_in_unsafe/default/test.rs  | 15 ++++++++++++-
 .../default/test.stderr                       | 16 +++++++++++---
 3 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/clippy_lints/src/macro_metavars_in_unsafe.rs b/clippy_lints/src/macro_metavars_in_unsafe.rs
index fed58f7ff14..e215097142b 100644
--- a/clippy_lints/src/macro_metavars_in_unsafe.rs
+++ b/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 @@ 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();
diff --git a/tests/ui-toml/macro_metavars_in_unsafe/default/test.rs b/tests/ui-toml/macro_metavars_in_unsafe/default/test.rs
index a312df5a43a..3dafea56514 100644
--- a/tests/ui-toml/macro_metavars_in_unsafe/default/test.rs
+++ b/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);
diff --git a/tests/ui-toml/macro_metavars_in_unsafe/default/test.stderr b/tests/ui-toml/macro_metavars_in_unsafe/default/test.stderr
index d6b97f6fde1..6f0ebcbba02 100644
--- a/tests/ui-toml/macro_metavars_in_unsafe/default/test.stderr
+++ b/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