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 Mar 2, 2024
1 parent e612d07 commit 1224847
Show file tree
Hide file tree
Showing 4 changed files with 250 additions and 9 deletions.
98 changes: 89 additions & 9 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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;
Expand Down Expand Up @@ -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(
Expand Down
57 changes: 57 additions & 0 deletions tests/ui/borrowck/dbg-issue-120327.rs
Original file line number Diff line number Diff line change
@@ -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() {}
100 changes: 100 additions & 0 deletions tests/ui/borrowck/dbg-issue-120327.stderr
Original file line number Diff line number Diff line change
@@ -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`.
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 1224847

Please sign in to comment.