Suggest a borrow when using dbg

chenyukang · chenyukang · commit 12248475b798 · 2024-03-02T10:23:38.000+08:00
diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
@@ -4,6 +4,7 @@
 #![allow(rustc::untranslatable_diagnostic)]
 
 use either::Either;
+use hir::Path;
 use rustc_data_structures::captures::Captures;
 use rustc_data_structures::fx::FxIndexSet;
 use rustc_errors::{codes::*, struct_span_code_err, Applicability, Diag, MultiSpan};
@@ -27,11 +28,13 @@ use rustc_mir_dataflow::move_paths::{InitKind, MoveOutIndex, MovePathIndex};
 use rustc_span::def_id::LocalDefId;
 use rustc_span::hygiene::DesugaringKind;
 use rustc_span::symbol::{kw, sym, Ident};
+use rustc_span::FileName;
 use rustc_span::{BytePos, Span, Symbol};
 use rustc_trait_selection::infer::InferCtxtExt;
 use rustc_trait_selection::traits::error_reporting::FindExprBySpan;
 use rustc_trait_selection::traits::ObligationCtxt;
 use std::iter;
+use std::path::PathBuf;
 
 use crate::borrow_set::TwoPhaseActivation;
 use crate::borrowck_errors;
@@ -463,19 +466,96 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                     self.suggest_cloning(err, ty, expr, move_span);
                 }
             }
+
+            self.suggest_ref_for_dbg_args(expr, span, move_span, err);
+
             if let Some(pat) = finder.pat {
-                *in_pattern = true;
-                let mut sugg = vec![(pat.span.shrink_to_lo(), "ref ".to_string())];
-                if let Some(pat) = finder.parent_pat {
-                    sugg.insert(0, (pat.span.shrink_to_lo(), "ref ".to_string()));
-                }
-                err.multipart_suggestion_verbose(
-                    "borrow this binding in the pattern to avoid moving the value",
-                    sugg,
-                    Applicability::MachineApplicable,
+                // FIXME: any better way to check this?
+                let from_std = self.infcx.tcx.sess.opts.real_rust_source_base_dir.clone().map_or(
+                    false,
+                    |root| {
+                        let file_path =
+                            match self.infcx.tcx.sess.source_map().span_to_filename(move_span) {
+                                FileName::Real(name) => {
+                                    name.clone().into_local_path().unwrap_or_default()
+                                }
+                                other => PathBuf::from(other.prefer_local().to_string()),
+                            };
+                        file_path.starts_with(&root.join("library/std/"))
+                    },
                 );
+                // it's useless to suggest inserting `ref` when the span comes from std library
+                // anyway, user can not modify std library in most cases, so let's keep it quite?
+                if !from_std {
+                    *in_pattern = true;
+                    let mut sugg = vec![(pat.span.shrink_to_lo(), "ref ".to_string())];
+                    if let Some(pat) = finder.parent_pat {
+                        sugg.insert(0, (pat.span.shrink_to_lo(), "ref ".to_string()));
+                    }
+                    err.multipart_suggestion_verbose(
+                        "borrow this binding in the pattern to avoid moving the value",
+                        sugg,
+                        Applicability::MachineApplicable,
+                    );
+                }
+            }
+        }
+    }
+
+    // for dbg!(x) which may take onwership, suggest dbg!(&x) instead
+    // but here we actually does not checking 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<'_>,
+        span: Option<Span>,
+        move_span: Span,
+        err: &mut Diag<'tcx>,
+    ) {
+        // only suggest for macro
+        if move_span.source_callsite() == move_span {
+            return;
+        }
+        let sm = self.infcx.tcx.sess.source_map();
+        let arg_code = if let Some(span) = span
+            && let Ok(code) = sm.span_to_snippet(span)
+        {
+            code
+        } else {
+            return;
+        };
+        struct MatchArgFinder {
+            expr_span: Span,
+            match_arg_span: Option<Span>,
+            arg_code: String,
+        }
+        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.as_str() == &self.arg_code
+                    && 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_code };
+        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(
diff --git a/tests/ui/borrowck/dbg-issue-120327.rs b/tests/ui/borrowck/dbg-issue-120327.rs
@@ -0,0 +1,57 @@
+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 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() {}
diff --git a/tests/ui/borrowck/dbg-issue-120327.stderr b/tests/ui/borrowck/dbg-issue-120327.stderr
@@ -0,0 +1,100 @@
+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]: borrow of moved value: `a`
+  --> $DIR/dbg-issue-120327.rs:54: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 6 previous errors
+
+For more information about this error, try `rustc --explain E0382`.
diff --git 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
 

Original file line number	Diff line number	Diff line change
`@@ -9,6 +9,10 @@ LL \| let _ = dbg!(a);`
`9`	`9`	`\| ^^^^^^^ value used here after move`
`10`	`10`	`\|`
`11`	`11`	= note: this error originates in the macro `dbg` (in Nightly builds, run with -Z macro-backtrace for more info)
	`12`	`+help: consider borrowing instead of transferring ownership`
	`13`	`+ \|`
	`14`	`+LL \| let _ = dbg!(&a);`
	`15`	`+ \| +`
`12`	`16`
`13`	`17`	`error: aborting due to 1 previous error`
`14`	`18`