Auto merge of #109944 - cjgillot:lint-cstring-macro, r=Nilstrieb

Do not suppress temporary_cstring_as_ptr in macros. There isn't really a reason to skip the lint when part of the expression comes from an expansion. Fixes #94694
rust-lang · Apr 4, 2023 · 6032700 · 6032700
2 parents 700938c + 8528ac6
commit 6032700
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 43 deletions.
diff --git a/compiler/rustc_lint/src/methods.rs b/compiler/rustc_lint/src/methods.rs
@@ -2,9 +2,9 @@ use crate::lints::CStringPtr;
 use crate::LateContext;
 use crate::LateLintPass;
 use crate::LintContext;
-use rustc_hir::{Expr, ExprKind, PathSegment};
+use rustc_hir::{Expr, ExprKind};
 use rustc_middle::ty;
-use rustc_span::{symbol::sym, ExpnKind, Span};
+use rustc_span::{symbol::sym, Span};
 
 declare_lint! {
     /// The `temporary_cstring_as_ptr` lint detects getting the inner pointer of
@@ -34,47 +34,14 @@ declare_lint! {
 
 declare_lint_pass!(TemporaryCStringAsPtr => [TEMPORARY_CSTRING_AS_PTR]);
 
-fn in_macro(span: Span) -> bool {
-    if span.from_expansion() {
-        !matches!(span.ctxt().outer_expn_data().kind, ExpnKind::Desugaring(..))
-    } else {
-        false
-    }
-}
-
-fn first_method_call<'tcx>(
-    expr: &'tcx Expr<'tcx>,
-) -> Option<(&'tcx PathSegment<'tcx>, &'tcx Expr<'tcx>)> {
-    if let ExprKind::MethodCall(path, receiver, args, ..) = &expr.kind {
-        if args.iter().any(|e| e.span.from_expansion()) || receiver.span.from_expansion() {
-            None
-        } else {
-            Some((path, *receiver))
-        }
-    } else {
-        None
-    }
-}
-
 impl<'tcx> LateLintPass<'tcx> for TemporaryCStringAsPtr {
     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
-        if in_macro(expr.span) {
-            return;
-        }
-
-        match first_method_call(expr) {
-            Some((path, unwrap_arg)) if path.ident.name == sym::as_ptr => {
-                let as_ptr_span = path.ident.span;
-                match first_method_call(unwrap_arg) {
-                    Some((path, receiver))
-                        if path.ident.name == sym::unwrap || path.ident.name == sym::expect =>
-                    {
-                        lint_cstring_as_ptr(cx, as_ptr_span, receiver, unwrap_arg);
-                    }
-                    _ => return,
-                }
-            }
-            _ => return,
+        if let ExprKind::MethodCall(as_ptr_path, as_ptr_receiver, ..) = expr.kind
+            && as_ptr_path.ident.name == sym::as_ptr
+            && let ExprKind::MethodCall(unwrap_path, unwrap_receiver, ..) = as_ptr_receiver.kind
+            && (unwrap_path.ident.name == sym::unwrap || unwrap_path.ident.name == sym::expect)
+        {
+            lint_cstring_as_ptr(cx, as_ptr_path.ident.span, unwrap_receiver, as_ptr_receiver);
         }
     }
 }

diff --git a/tests/ui/lint/lint-temporary-cstring-as-ptr.rs b/tests/ui/lint/lint-temporary-cstring-as-ptr.rs
@@ -3,7 +3,15 @@
 
 use std::ffi::CString;
 
+macro_rules! mymacro {
+    () => {
+        let s = CString::new("some text").unwrap().as_ptr();
+        //~^ ERROR getting the inner pointer of a temporary `CString`
+    }
+}
+
 fn main() {
     let s = CString::new("some text").unwrap().as_ptr();
     //~^ ERROR getting the inner pointer of a temporary `CString`
+    mymacro!();
 }
diff --git a/tests/ui/lint/lint-temporary-cstring-as-ptr.stderr b/tests/ui/lint/lint-temporary-cstring-as-ptr.stderr
@@ -1,5 +1,5 @@
 error: getting the inner pointer of a temporary `CString`
-  --> $DIR/lint-temporary-cstring-as-ptr.rs:7:48
+  --> $DIR/lint-temporary-cstring-as-ptr.rs:14:48
    |
 LL |     let s = CString::new("some text").unwrap().as_ptr();
    |             ---------------------------------- ^^^^^^ this pointer will be invalid
@@ -14,5 +14,20 @@ note: the lint level is defined here
 LL | #![deny(temporary_cstring_as_ptr)]
    |         ^^^^^^^^^^^^^^^^^^^^^^^^
 
-error: aborting due to previous error
+error: getting the inner pointer of a temporary `CString`
+  --> $DIR/lint-temporary-cstring-as-ptr.rs:8:52
+   |
+LL |         let s = CString::new("some text").unwrap().as_ptr();
+   |                 ---------------------------------- ^^^^^^ this pointer will be invalid
+   |                 |
+   |                 this `CString` is deallocated at the end of the statement, bind it to a variable to extend its lifetime
+...
+LL |     mymacro!();
+   |     ---------- in this macro invocation
+   |
+   = note: pointers do not have a lifetime; when calling `as_ptr` the `CString` will be deallocated at the end of the statement because nothing is referencing it as far as the type system is concerned
+   = help: for more information, see https://doc.rust-lang.org/reference/destructors.html
+   = note: this error originates in the macro `mymacro` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: aborting due to 2 previous errors