From 12248475b798cc7329c8b4665a6e07d94c84100f Mon Sep 17 00:00:00 2001 From: yukang Date: Tue, 13 Feb 2024 05:24:32 +0800 Subject: [PATCH] Suggest a borrow when using dbg --- .../src/diagnostics/conflict_errors.rs | 98 +++++++++++++++-- tests/ui/borrowck/dbg-issue-120327.rs | 57 ++++++++++ tests/ui/borrowck/dbg-issue-120327.stderr | 100 ++++++++++++++++++ .../dbg-macro-move-semantics.stderr | 4 + 4 files changed, 250 insertions(+), 9 deletions(-) create mode 100644 tests/ui/borrowck/dbg-issue-120327.rs create mode 100644 tests/ui/borrowck/dbg-issue-120327.stderr diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index 0776f455efd9c..9166152385cb9 100644 --- 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, + 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, + 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 new file mode 100644 index 0000000000000..08421a7ea220b --- /dev/null +++ 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 new file mode 100644 index 0000000000000..7f10cf4516ebf --- /dev/null +++ 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 index c2b9899e20db3..f515cb62c7cde 100644 --- 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