Rollup merge of #120990 - chenyukang:yukang-fix-120327-dbg, r=oli-obk

Suggest a borrow when using dbg Fixes #120327 r? ````@estebank````
2024-07-16 02:02:23 -05:00 · 2024-07-16 02:02:23 -05:00 · 12fd2f94b3
commit 12fd2f94b3
parent 5c84886056 317952677a
4 changed files with 252 additions and 2 deletions
--- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
+++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
@ -4,7 +4,7 @@
 #![allow(rustc::untranslatable_diagnostic)]

 use either::Either;
-use hir::ClosureKind;
+use hir::{ClosureKind, Path};
 use rustc_data_structures::captures::Captures;
 use rustc_data_structures::fx::FxIndexSet;
 use rustc_errors::{codes::*, struct_span_code_err, Applicability, Diag, MultiSpan};
@ -16,6 +16,7 @@
 use rustc_middle::bug;
 use rustc_middle::hir::nested_filter::OnlyBodies;
 use rustc_middle::mir::tcx::PlaceTy;
+use rustc_middle::mir::VarDebugInfoContents;
 use rustc_middle::mir::{
    self, AggregateKind, BindingForm, BorrowKind, CallSource, ClearCrossCrate, ConstraintCategory,
    FakeBorrowKind, FakeReadCause, LocalDecl, LocalInfo, LocalKind, Location, MutBorrowKind,
@ -546,7 +547,14 @@ fn visit_pat(&mut self, p: &'hir hir::Pat<'hir>) {
                    self.suggest_cloning(err, ty, expr, None, Some(move_spans));
                }
            }
-            if let Some(pat) = finder.pat {
+
+            self.suggest_ref_for_dbg_args(expr, place, move_span, err);
+
+            // it's useless to suggest inserting `ref` when the span don't comes from local code
+            if let Some(pat) = finder.pat
+                && !move_span.is_dummy()
+                && !self.infcx.tcx.sess.source_map().is_imported(move_span)
+            {
                *in_pattern = true;
                let mut sugg = vec![(pat.span.shrink_to_lo(), "ref ".to_string())];
                if let Some(pat) = finder.parent_pat {
@ -561,6 +569,59 @@ fn visit_pat(&mut self, p: &'hir hir::Pat<'hir>) {
        }
    }

+    // for dbg!(x) which may take ownership, suggest dbg!(&x) instead
+    // but here we actually do not check whether the macro name is `dbg!`
+    // so that we may extend the scope a bit larger to cover more cases
+    fn suggest_ref_for_dbg_args(
+        &self,
+        body: &hir::Expr<'_>,
+        place: &Place<'tcx>,
+        move_span: Span,
+        err: &mut Diag<'infcx>,
+    ) {
+        let var_info = self.body.var_debug_info.iter().find(|info| match info.value {
+            VarDebugInfoContents::Place(ref p) => p == place,
+            _ => false,
+        });
+        let arg_name = if let Some(var_info) = var_info {
+            var_info.name
+        } else {
+            return;
+        };
+        struct MatchArgFinder {
+            expr_span: Span,
+            match_arg_span: Option<Span>,
+            arg_name: Symbol,
+        }
+        impl Visitor<'_> for MatchArgFinder {
+            fn visit_expr(&mut self, e: &hir::Expr<'_>) {
+                // dbg! is expanded into a match pattern, we need to find the right argument span
+                if let hir::ExprKind::Match(expr, ..) = &e.kind
+                    && let hir::ExprKind::Path(hir::QPath::Resolved(
+                        _,
+                        path @ Path { segments: [seg], .. },
+                    )) = &expr.kind
+                    && seg.ident.name == self.arg_name
+                    && self.expr_span.source_callsite().contains(expr.span)
+                {
+                    self.match_arg_span = Some(path.span);
+                }
+                hir::intravisit::walk_expr(self, e);
+            }
+        }
+
+        let mut finder = MatchArgFinder { expr_span: move_span, match_arg_span: None, arg_name };
+        finder.visit_expr(body);
+        if let Some(macro_arg_span) = finder.match_arg_span {
+            err.span_suggestion_verbose(
+                macro_arg_span.shrink_to_lo(),
+                "consider borrowing instead of transferring ownership",
+                "&",
+                Applicability::MachineApplicable,
+            );
+        }
+    }
+
    fn report_use_of_uninitialized(
        &self,
        mpi: MovePathIndex,
--- a/tests/ui/borrowck/dbg-issue-120327.rs
+++ b/tests/ui/borrowck/dbg-issue-120327.rs
@ -0,0 +1,68 @@
+fn s() -> String {
+    let a = String::new();
+    dbg!(a);
+    return a; //~ ERROR use of moved value:
+}
+
+fn m() -> String {
+    let a = String::new();
+    dbg!(1, 2, a, 1, 2);
+    return a; //~ ERROR use of moved value:
+}
+
+fn t(a: String) -> String {
+    let b: String = "".to_string();
+    dbg!(a, b);
+    return b; //~ ERROR use of moved value:
+}
+
+fn x(a: String) -> String {
+    let b: String = "".to_string();
+    dbg!(a, b);
+    return a; //~ ERROR use of moved value:
+}
+
+macro_rules! my_dbg {
+    () => {
+        eprintln!("[{}:{}:{}]", file!(), line!(), column!())
+    };
+    ($val:expr $(,)?) => {
+        match $val {
+            tmp => {
+                eprintln!("[{}:{}:{}] {} = {:#?}",
+                    file!(), line!(), column!(), stringify!($val), &tmp);
+                tmp
+            }
+        }
+    };
+    ($($val:expr),+ $(,)?) => {
+        ($(my_dbg!($val)),+,)
+    };
+}
+
+fn test_my_dbg() -> String {
+    let b: String = "".to_string();
+    my_dbg!(b, 1);
+    return b; //~ ERROR use of moved value:
+}
+
+fn test_not_macro() -> String {
+    let a = String::new();
+    let _b = match a {
+        tmp => {
+            eprintln!("dbg: {}", tmp);
+            tmp
+        }
+    };
+    return a; //~ ERROR use of moved value:
+}
+
+fn get_expr(_s: String) {}
+
+fn test() {
+    let a: String = "".to_string();
+    let _res = get_expr(dbg!(a));
+    let _l = a.len(); //~ ERROR borrow of moved value
+}
+
+fn main() {}
--- a/tests/ui/borrowck/dbg-issue-120327.stderr
+++ b/tests/ui/borrowck/dbg-issue-120327.stderr
@ -0,0 +1,117 @@
+error[E0382]: use of moved value: `a`
+  --> $DIR/dbg-issue-120327.rs:4:12
+   |
+LL |     let a = String::new();
+   |         - move occurs because `a` has type `String`, which does not implement the `Copy` trait
+LL |     dbg!(a);
+   |     ------- value moved here
+LL |     return a;
+   |            ^ value used here after move
+   |
+help: consider borrowing instead of transferring ownership
+   |
+LL |     dbg!(&a);
+   |          +
+
+error[E0382]: use of moved value: `a`
+  --> $DIR/dbg-issue-120327.rs:10:12
+   |
+LL |     let a = String::new();
+   |         - move occurs because `a` has type `String`, which does not implement the `Copy` trait
+LL |     dbg!(1, 2, a, 1, 2);
+   |     ------------------- value moved here
+LL |     return a;
+   |            ^ value used here after move
+   |
+help: consider borrowing instead of transferring ownership
+   |
+LL |     dbg!(1, 2, &a, 1, 2);
+   |                +
+
+error[E0382]: use of moved value: `b`
+  --> $DIR/dbg-issue-120327.rs:16:12
+   |
+LL |     let b: String = "".to_string();
+   |         - move occurs because `b` has type `String`, which does not implement the `Copy` trait
+LL |     dbg!(a, b);
+   |     ---------- value moved here
+LL |     return b;
+   |            ^ value used here after move
+   |
+help: consider borrowing instead of transferring ownership
+   |
+LL |     dbg!(a, &b);
+   |             +
+
+error[E0382]: use of moved value: `a`
+  --> $DIR/dbg-issue-120327.rs:22:12
+   |
+LL | fn x(a: String) -> String {
+   |      - move occurs because `a` has type `String`, which does not implement the `Copy` trait
+LL |     let b: String = "".to_string();
+LL |     dbg!(a, b);
+   |     ---------- value moved here
+LL |     return a;
+   |            ^ value used here after move
+   |
+help: consider borrowing instead of transferring ownership
+   |
+LL |     dbg!(&a, b);
+   |          +
+
+error[E0382]: use of moved value: `b`
+  --> $DIR/dbg-issue-120327.rs:46:12
+   |
+LL |             tmp => {
+   |             --- value moved here
+...
+LL |     let b: String = "".to_string();
+   |         - move occurs because `b` has type `String`, which does not implement the `Copy` trait
+LL |     my_dbg!(b, 1);
+LL |     return b;
+   |            ^ value used here after move
+   |
+help: consider borrowing instead of transferring ownership
+   |
+LL |     my_dbg!(&b, 1);
+   |             +
+help: borrow this binding in the pattern to avoid moving the value
+   |
+LL |             ref tmp => {
+   |             +++
+
+error[E0382]: use of moved value: `a`
+  --> $DIR/dbg-issue-120327.rs:57:12
+   |
+LL |     let a = String::new();
+   |         - move occurs because `a` has type `String`, which does not implement the `Copy` trait
+LL |     let _b = match a {
+LL |         tmp => {
+   |         --- value moved here
+...
+LL |     return a;
+   |            ^ value used here after move
+   |
+help: borrow this binding in the pattern to avoid moving the value
+   |
+LL |         ref tmp => {
+   |         +++
+
+error[E0382]: borrow of moved value: `a`
+  --> $DIR/dbg-issue-120327.rs:65:14
+   |
+LL |     let a: String = "".to_string();
+   |         - move occurs because `a` has type `String`, which does not implement the `Copy` trait
+LL |     let _res = get_expr(dbg!(a));
+   |                         ------- value moved here
+LL |     let _l = a.len();
+   |              ^ value borrowed here after move
+   |
+help: consider borrowing instead of transferring ownership
+   |
+LL |     let _res = get_expr(dbg!(&a));
+   |                              +
+
+error: aborting due to 7 previous errors
+
+For more information about this error, try `rustc --explain E0382`.
--- a/tests/ui/rfcs/rfc-2361-dbg-macro/dbg-macro-move-semantics.stderr
+++ b/tests/ui/rfcs/rfc-2361-dbg-macro/dbg-macro-move-semantics.stderr
@ -9,6 +9,10 @@ LL |     let _ = dbg!(a);
   |             ^^^^^^^ value used here after move
   |
   = note: this error originates in the macro `dbg` (in Nightly builds, run with -Z macro-backtrace for more info)
+help: consider borrowing instead of transferring ownership
+   |
+LL |     let _ = dbg!(&a);
+   |                  +

 error: aborting due to 1 previous error