Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggest a borrow when using dbg #120990

Merged
merged 1 commit into from
Jul 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -546,7 +547,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 @@ -561,6 +569,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
Loading