Skip to content

Remove the unhelpful let binding diag comes from FormatArguments #114511

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

Merged
merged 1 commit into from
Sep 6, 2023
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
50 changes: 37 additions & 13 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2130,21 +2130,27 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
/// misleading users in cases like `tests/ui/nll/borrowed-temporary-error.rs`.
/// We could expand the analysis to suggest hoising all of the relevant parts of
/// the users' code to make the code compile, but that could be too much.
struct NestedStatementVisitor {
/// We found the `prop_expr` by the way to check whether the expression is a `FormatArguments`,
/// which is a special case since it's generated by the compiler.
struct NestedStatementVisitor<'tcx> {
span: Span,
current: usize,
found: usize,
prop_expr: Option<&'tcx hir::Expr<'tcx>>,
}

impl<'tcx> Visitor<'tcx> for NestedStatementVisitor {
fn visit_block(&mut self, block: &hir::Block<'tcx>) {
impl<'tcx> Visitor<'tcx> for NestedStatementVisitor<'tcx> {
fn visit_block(&mut self, block: &'tcx hir::Block<'tcx>) {
self.current += 1;
walk_block(self, block);
self.current -= 1;
}
fn visit_expr(&mut self, expr: &hir::Expr<'tcx>) {
fn visit_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) {
if self.span == expr.span.source_callsite() {
self.found = self.current;
if self.prop_expr.is_none() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if we have nested format_args expressions?

Do we only care about the case where we assign the result of format_args!? If so, why can't we just check for stmt being an assignment where the rhs expr is_format_args_item?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the if self.span == expr.span.source_callsite() guarantee we find out the expr we're intrested in, the code goes here means a borrow(mostly binding) happens, I think use span to find expr is mostly enough for suggestions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does your branch output on something like this: let x = format_args!("a {} {} {}.", 1, format_args!("b{}!", 2), 3)? Can you maybe add this to your test?

Copy link
Member Author

@chenyukang chenyukang Aug 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The output is:

error[E0716]: temporary value dropped while borrowed
 --> tests/ui/borrowck/issue-114374-invalid-help-fmt-args.rs:5:13
  |
5 |     let x = format_args!("a {} {} {}.", 1, format_args!("b{}!", 2), 3);
  |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- temporary value is freed at the end of this statement
  |             |
  |             creates a temporary value which is freed while still in use
6 |     bar(x);
  |         - borrow later used here
  |
  = note: the result of `format_args!` can only be assigned directly if no placeholders in it's arguments are used
  = note: to learn more, visit <https://doc.rust-lang.org/std/macro.format_args.html>
  = note: this error originates in the macro `format_args` (in Nightly builds, run with -Z macro-backtrace for more info)

I added it to the test case.

self.prop_expr = Some(expr);
}
}
walk_expr(self, expr);
}
Expand All @@ -2162,22 +2168,40 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
span: proper_span,
current: 0,
found: 0,
prop_expr: None,
};
visitor.visit_stmt(stmt);

let typeck_results = self.infcx.tcx.typeck(self.mir_def_id());
let expr_ty: Option<Ty<'_>> = visitor.prop_expr.map(|expr| typeck_results.expr_ty(expr).peel_refs());

let is_format_arguments_item =
if let Some(expr_ty) = expr_ty
&& let ty::Adt(adt, _) = expr_ty.kind() {
self.infcx.tcx.lang_items().get(LangItem::FormatArguments) == Some(adt.did())
} else {
false
};

if visitor.found == 0
&& stmt.span.contains(proper_span)
&& let Some(p) = sm.span_to_margin(stmt.span)
&& let Ok(s) = sm.span_to_snippet(proper_span)
{
let addition = format!("let binding = {};\n{}", s, " ".repeat(p));
err.multipart_suggestion_verbose(
msg,
vec![
(stmt.span.shrink_to_lo(), addition),
(proper_span, "binding".to_string()),
],
Applicability::MaybeIncorrect,
);
if !is_format_arguments_item {
let addition = format!("let binding = {};\n{}", s, " ".repeat(p));
err.multipart_suggestion_verbose(
msg,
vec![
(stmt.span.shrink_to_lo(), addition),
(proper_span, "binding".to_string()),
],
Applicability::MaybeIncorrect,
);
} else {
err.note("the result of `format_args!` can only be assigned directly if no placeholders in it's arguments are used");
err.note("to learn more, visit <https://doc.rust-lang.org/std/macro.format_args.html>");
}
suggested = true;
break;
}
Expand Down
16 changes: 16 additions & 0 deletions tests/ui/borrowck/issue-114374-invalid-help-fmt-args.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#![allow(dead_code)]

fn bar<'a>(_: std::fmt::Arguments<'a>) {}
fn main() {
let x = format_args!("a {} {} {}.", 1, format_args!("b{}!", 2), 3);
//~^ ERROR temporary value dropped while borrowed

bar(x);

let foo = format_args!("{}", "hi");
//~^ ERROR temporary value dropped while borrowed
bar(foo);

let foo = format_args!("hi"); // no placeholder in arguments, so no error
bar(foo);
}
33 changes: 33 additions & 0 deletions tests/ui/borrowck/issue-114374-invalid-help-fmt-args.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
error[E0716]: temporary value dropped while borrowed
--> $DIR/issue-114374-invalid-help-fmt-args.rs:5:13
|
LL | let x = format_args!("a {} {} {}.", 1, format_args!("b{}!", 2), 3);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- temporary value is freed at the end of this statement
| |
| creates a temporary value which is freed while still in use
...
LL | bar(x);
| - borrow later used here
|
= note: the result of `format_args!` can only be assigned directly if no placeholders in it's arguments are used
= note: to learn more, visit <https://doc.rust-lang.org/std/macro.format_args.html>
= note: this error originates in the macro `format_args` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0716]: temporary value dropped while borrowed
--> $DIR/issue-114374-invalid-help-fmt-args.rs:10:15
|
LL | let foo = format_args!("{}", "hi");
| ^^^^^^^^^^^^^^^^^^^^^^^^- temporary value is freed at the end of this statement
| |
| creates a temporary value which is freed while still in use
LL |
LL | bar(foo);
| --- borrow later used here
|
= note: the result of `format_args!` can only be assigned directly if no placeholders in it's arguments are used
= note: to learn more, visit <https://doc.rust-lang.org/std/macro.format_args.html>
= note: this error originates in the macro `format_args` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0716`.