Skip to content

Commit

Permalink
Suggest a borrow when using dbg
Browse files Browse the repository at this point in the history
  • Loading branch information
chenyukang committed Jul 4, 2024
1 parent 9ffe52e commit 85e312f
Show file tree
Hide file tree
Showing 4 changed files with 252 additions and 2 deletions.
65 changes: 63 additions & 2 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -16,6 +16,7 @@ use rustc_hir::{CoroutineKind, CoroutineSource, LangItem};
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,
Expand Down Expand Up @@ -494,7 +495,14 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, '_, 'infcx, 'tcx> {
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 {
Expand All @@ -509,6 +517,59 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, '_, 'infcx, 'tcx> {
}
}

// 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,
Expand Down
68 changes: 68 additions & 0 deletions tests/ui/borrowck/dbg-issue-120327.rs
Original file line number Diff line number Diff line change
@@ -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() {}
117 changes: 117 additions & 0 deletions tests/ui/borrowck/dbg-issue-120327.stderr
Original file line number Diff line number Diff line change
@@ -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`.
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit 85e312f

Please sign in to comment.