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.

Sign up for GitHub

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

Jump to bottom

E0277: suggest dereferencing function arguments in more cases #133292

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+206 −113
Open

E0277: suggest dereferencing function arguments in more cases #133292

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
198 changes: 85 additions & 113 deletions compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -443,9 +443,8 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
}
}

/// When after several dereferencing, the reference satisfies the trait
/// bound. This function provides dereference suggestion for this
/// specific situation.
/// Provide a suggestion to dereference arguments to functions and binary operators, if that
/// would satisfy trait bounds.
pub(super) fn suggest_dereferences(
&self,
obligation: &PredicateObligation<'tcx>,
Expand All @@ -459,127 +458,100 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
&& let Some(arg_ty) = typeck_results.expr_ty_adjusted_opt(expr)
{
// Suggest dereferencing the argument to a function/method call if possible

// Get the root obligation, since the leaf obligation we have may be unhelpful (#87437)
let mut real_trait_pred = trait_pred;
while let Some((parent_code, parent_trait_pred)) = code.parent() {
code = parent_code;
if let Some(parent_trait_pred) = parent_trait_pred {
real_trait_pred = parent_trait_pred;
}
}

// We `instantiate_bound_regions_with_erased` here because `make_subregion` does not handle
// `ReBound`, and we don't particularly care about the regions.
let real_ty =
self.tcx.instantiate_bound_regions_with_erased(real_trait_pred.self_ty());
// We `instantiate_bound_regions_with_erased` here because `make_subregion` does not handle
// `ReBound`, and we don't particularly care about the regions.
let real_ty = self.tcx.instantiate_bound_regions_with_erased(real_trait_pred.self_ty());
if !self.can_eq(obligation.param_env, real_ty, arg_ty) {
return false;
}

if self.can_eq(obligation.param_env, real_ty, arg_ty)
&& let ty::Ref(region, base_ty, mutbl) = *real_ty.kind()
// Potentially, we'll want to place our dereferences under a `&`. We don't try this for
// `&mut`, since we can't be sure users will get the side-effects they want from it.
// If this doesn't work, we'll try removing the `&` in `suggest_remove_reference`.
// FIXME(dianne): this misses the case where users need both to deref and remove `&`s.
// This method could be combined with `TypeErrCtxt::suggest_remove_reference` to handle
// that, similar to what `FnCtxt::suggest_deref_or_ref` does.
let (is_under_ref, base_ty, span) = match expr.kind {
hir::ExprKind::AddrOf(hir::BorrowKind::Ref, hir::Mutability::Not, subexpr)
if let &ty::Ref(region, base_ty, hir::Mutability::Not) = real_ty.kind() =>
{
let autoderef = (self.autoderef_steps)(base_ty);
if let Some(steps) =
autoderef.into_iter().enumerate().find_map(|(steps, (ty, obligations))| {
// Re-add the `&`
let ty = Ty::new_ref(self.tcx, region, ty, mutbl);

// Remapping bound vars here
let real_trait_pred_and_ty = real_trait_pred
.map_bound(|inner_trait_pred| (inner_trait_pred, ty));
let obligation = self.mk_trait_obligation_with_new_self_ty(
obligation.param_env,
real_trait_pred_and_ty,
);
let may_hold = obligations
.iter()
.chain([&obligation])
.all(|obligation| self.predicate_may_hold(obligation))
.then_some(steps);
(Some(region), base_ty, subexpr.span)
}
// Don't suggest `*&mut`, etc.
hir::ExprKind::AddrOf(..) => return false,
_ => (None, real_ty, obligation.cause.span),
};

may_hold
})
{
if steps > 0 {
// Don't care about `&mut` because `DerefMut` is used less
// often and user will not expect that an autoderef happens.
if let hir::Node::Expr(hir::Expr {
kind:
hir::ExprKind::AddrOf(
hir::BorrowKind::Ref,
hir::Mutability::Not,
expr,
),
..
}) = self.tcx.hir_node(*arg_hir_id)
{
let derefs = "*".repeat(steps);
err.span_suggestion_verbose(
expr.span.shrink_to_lo(),
"consider dereferencing here",
derefs,
Applicability::MachineApplicable,
);
return true;
}
}
} else if real_trait_pred != trait_pred {
// This branch addresses #87437.

let span = obligation.cause.span;
// Remapping bound vars here
let real_trait_pred_and_base_ty = real_trait_pred
.map_bound(|inner_trait_pred| (inner_trait_pred, base_ty));
let obligation = self.mk_trait_obligation_with_new_self_ty(
obligation.param_env,
real_trait_pred_and_base_ty,
);
let sized_obligation = Obligation::new(
self.tcx,
obligation.cause.clone(),
obligation.param_env,
ty::TraitRef::new(
self.tcx,
self.tcx.require_lang_item(
hir::LangItem::Sized,
Some(obligation.cause.span),
),
[base_ty],
),
);
if self.predicate_may_hold(&obligation)
&& self.predicate_must_hold_modulo_regions(&sized_obligation)
// Do not suggest * if it is already a reference,
// will suggest removing the borrow instead in that case.
&& !matches!(expr.kind, hir::ExprKind::AddrOf(..))
{
let call_node = self.tcx.hir_node(*call_hir_id);
let msg = "consider dereferencing here";
let is_receiver = matches!(
call_node,
Node::Expr(hir::Expr {
kind: hir::ExprKind::MethodCall(_, receiver_expr, ..),
..
})
if receiver_expr.hir_id == *arg_hir_id
);
if is_receiver {
err.multipart_suggestion_verbose(
msg,
vec![
(span.shrink_to_lo(), "(*".to_string()),
(span.shrink_to_hi(), ")".to_string()),
],
Applicability::MachineApplicable,
)
} else {
err.span_suggestion_verbose(
span.shrink_to_lo(),
msg,
'*',
Applicability::MachineApplicable,
)
};
return true;
}
}
let autoderef = (self.autoderef_steps)(base_ty);
let mut is_boxed = base_ty.is_box();
if let Some(steps) = autoderef.into_iter().position(|(mut ty, obligations)| {
// Ensure one of the following for dereferencing to be valid: we're passing by
// reference, `ty` is `Copy`, or we're moving out of a (potentially nested) `Box`.
let can_deref = is_under_ref.is_some()
|| self.type_is_copy_modulo_regions(obligation.param_env, ty)
|| ty.is_numeric() // for inference vars (presumably but not provably `Copy`)
|| is_boxed && self.type_is_sized_modulo_regions(obligation.param_env, ty);
is_boxed &= ty.is_box();

// Re-add the `&` if necessary
if let Some(region) = is_under_ref {
ty = Ty::new_ref(self.tcx, region, ty, hir::Mutability::Not);
}

// Remapping bound vars here
let real_trait_pred_and_ty =
real_trait_pred.map_bound(|inner_trait_pred| (inner_trait_pred, ty));
let obligation = self.mk_trait_obligation_with_new_self_ty(
obligation.param_env,
real_trait_pred_and_ty,
);

can_deref
&& obligations
.iter()
.chain([&obligation])
.all(|obligation| self.predicate_may_hold(obligation))
}) && steps > 0
{
let derefs = "*".repeat(steps);
let msg = "consider dereferencing here";
let call_node = self.tcx.hir_node(*call_hir_id);
let is_receiver = matches!(
call_node,
Node::Expr(hir::Expr {
kind: hir::ExprKind::MethodCall(_, receiver_expr, ..),
..
})
if receiver_expr.hir_id == *arg_hir_id
);
if is_receiver {
err.multipart_suggestion_verbose(
msg,
vec![
(span.shrink_to_lo(), format!("({derefs}")),
(span.shrink_to_hi(), ")".to_string()),
],
Applicability::MachineApplicable,
)
} else {
err.span_suggestion_verbose(
span.shrink_to_lo(),
msg,
derefs,
Applicability::MachineApplicable,
)
};
return true;
}
} else if let (
ObligationCauseCode::BinOp { lhs_hir_id, rhs_hir_id: Some(rhs_hir_id), .. },
Expand Down
4 changes: 4 additions & 0 deletions tests/ui/no_send-rc.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ note: required by a bound in `bar`
|
LL | fn bar<T: Send>(_: T) {}
| ^^^^ required by this bound in `bar`
help: consider dereferencing here
|
LL | bar(*x);
| +

error: aborting due to 1 previous error

Expand Down
4 changes: 4 additions & 0 deletions tests/ui/suggestions/issue-84973-blacklist.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,10 @@ note: required by a bound in `f_send`
|
LL | fn f_send<T: Send>(t: T) {}
| ^^^^ required by this bound in `f_send`
help: consider dereferencing here
|
LL | f_send(*rc);
| +

error: aborting due to 5 previous errors

Expand Down
37 changes: 37 additions & 0 deletions tests/ui/traits/suggest-dereferences/deref-argument.fixed
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
//@ run-rustfix
//! diagnostic test for #90997.
//! test that E0277 suggests dereferences to satisfy bounds when the referent is `Copy` or boxed.
use std::ops::Deref;

trait Test {
fn test(self);
}
fn consume_test(x: impl Test) { x.test() }

impl Test for u32 {
fn test(self) {}
}
struct MyRef(u32);
impl Deref for MyRef {
type Target = u32;
fn deref(&self) -> &Self::Target {
&self.0
}
}

struct NonCopy;
impl Test for NonCopy {
fn test(self) {}
}

fn main() {
let my_ref = MyRef(0);
consume_test(*my_ref);
//~^ ERROR the trait bound `MyRef: Test` is not satisfied
//~| SUGGESTION *

let nested_box = Box::new(Box::new(Box::new(NonCopy)));
consume_test(***nested_box);
//~^ ERROR the trait bound `Box<Box<Box<NonCopy>>>: Test` is not satisfied
//~| SUGGESTION ***
}
37 changes: 37 additions & 0 deletions tests/ui/traits/suggest-dereferences/deref-argument.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
//@ run-rustfix
//! diagnostic test for #90997.
//! test that E0277 suggests dereferences to satisfy bounds when the referent is `Copy` or boxed.
use std::ops::Deref;

trait Test {
fn test(self);
}
fn consume_test(x: impl Test) { x.test() }

impl Test for u32 {
fn test(self) {}
}
struct MyRef(u32);
impl Deref for MyRef {
type Target = u32;
fn deref(&self) -> &Self::Target {
&self.0
}
}

struct NonCopy;
impl Test for NonCopy {
fn test(self) {}
}

fn main() {
let my_ref = MyRef(0);
consume_test(my_ref);
//~^ ERROR the trait bound `MyRef: Test` is not satisfied
//~| SUGGESTION *

let nested_box = Box::new(Box::new(Box::new(NonCopy)));
consume_test(nested_box);
//~^ ERROR the trait bound `Box<Box<Box<NonCopy>>>: Test` is not satisfied
//~| SUGGESTION ***
}
39 changes: 39 additions & 0 deletions tests/ui/traits/suggest-dereferences/deref-argument.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
error[E0277]: the trait bound `MyRef: Test` is not satisfied
--> $DIR/deref-argument.rs:29:18
|
LL | consume_test(my_ref);
| ------------ ^^^^^^ the trait `Test` is not implemented for `MyRef`
| |
| required by a bound introduced by this call
|
note: required by a bound in `consume_test`
--> $DIR/deref-argument.rs:9:25
|
LL | fn consume_test(x: impl Test) { x.test() }
| ^^^^ required by this bound in `consume_test`
help: consider dereferencing here
|
LL | consume_test(*my_ref);
| +

error[E0277]: the trait bound `Box<Box<Box<NonCopy>>>: Test` is not satisfied
--> $DIR/deref-argument.rs:34:18
|
LL | consume_test(nested_box);
| ------------ ^^^^^^^^^^ the trait `Test` is not implemented for `Box<Box<Box<NonCopy>>>`
| |
| required by a bound introduced by this call
|
note: required by a bound in `consume_test`
--> $DIR/deref-argument.rs:9:25
|
LL | fn consume_test(x: impl Test) { x.test() }
| ^^^^ required by this bound in `consume_test`
help: consider dereferencing here
|
LL | consume_test(***nested_box);
| +++

error: aborting due to 2 previous errors

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