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

Point at argument instead of call for their obligations #88719

Merged
merged 10 commits into from
Sep 17, 2021
1 change: 0 additions & 1 deletion compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1998,7 +1998,6 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
&obligation,
&traits::SelectionError::Unimplemented,
false,
false,
);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,13 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
SubregionOrigin::Subtype(box TypeTrace { ref cause, .. }) => cause,
_ => return None,
};
let (parent, impl_def_id) = match &cause.code {
// If we added a "points at argument expression" obligation, we remove it here, we care
// about the original obligation only.
let code = match &cause.code {
ObligationCauseCode::FunctionArgumentObligation { parent_code, .. } => &*parent_code,
_ => &cause.code,
};
let (parent, impl_def_id) = match code {
ObligationCauseCode::MatchImpl(parent, impl_def_id) => (parent, impl_def_id),
_ => return None,
};
Expand Down
6 changes: 1 addition & 5 deletions compiler/rustc_infer/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,6 @@ pub type Selection<'tcx> = ImplSource<'tcx, PredicateObligation<'tcx>>;
pub struct FulfillmentError<'tcx> {
pub obligation: PredicateObligation<'tcx>,
pub code: FulfillmentErrorCode<'tcx>,
/// Diagnostics only: we opportunistically change the `code.span` when we encounter an
/// obligation error caused by a call argument. When this is the case, we also signal that in
/// this field to ensure accuracy of suggestions.
pub points_at_arg_span: bool,
/// Diagnostics only: the 'root' obligation which resulted in
/// the failure to process `obligation`. This is the obligation
/// that was initially passed to `register_predicate_obligation`
Expand Down Expand Up @@ -128,7 +124,7 @@ impl<'tcx> FulfillmentError<'tcx> {
code: FulfillmentErrorCode<'tcx>,
root_obligation: PredicateObligation<'tcx>,
) -> FulfillmentError<'tcx> {
FulfillmentError { obligation, code, points_at_arg_span: false, root_obligation }
FulfillmentError { obligation, code, root_obligation }
}
}

Expand Down
18 changes: 14 additions & 4 deletions compiler/rustc_middle/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,15 @@ pub enum ObligationCauseCode<'tcx> {

DerivedObligation(DerivedObligationCause<'tcx>),

FunctionArgumentObligation {
/// The node of the relevant argument in the function call.
arg_hir_id: hir::HirId,
/// The node of the function call.
call_hir_id: hir::HirId,
/// The obligation introduced by this argument.
parent_code: Lrc<ObligationCauseCode<'tcx>>,
},

/// Error derived when matching traits/impls; see ObligationCause for more details
CompareImplConstObligation,

Expand Down Expand Up @@ -368,11 +377,12 @@ impl ObligationCauseCode<'_> {
// Return the base obligation, ignoring derived obligations.
pub fn peel_derives(&self) -> &Self {
let mut base_cause = self;
while let BuiltinDerivedObligation(cause)
| ImplDerivedObligation(cause)
| DerivedObligation(cause) = base_cause
while let BuiltinDerivedObligation(DerivedObligationCause { parent_code, .. })
| ImplDerivedObligation(DerivedObligationCause { parent_code, .. })
| DerivedObligation(DerivedObligationCause { parent_code, .. })
| FunctionArgumentObligation { parent_code, .. } = base_cause
{
base_cause = &cause.parent_code;
base_cause = &parent_code;
}
base_cause
}
Expand Down
3 changes: 0 additions & 3 deletions compiler/rustc_trait_selection/src/traits/chalk_fulfill.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ impl TraitEngine<'tcx> for FulfillmentContext<'tcx> {
.map(|obligation| FulfillmentError {
obligation: obligation.clone(),
code: FulfillmentErrorCode::CodeAmbiguity,
points_at_arg_span: false,
// FIXME - does Chalk have a notation of 'root obligation'?
// This is just for diagnostics, so it's okay if this is wrong
root_obligation: obligation.clone(),
Expand Down Expand Up @@ -112,7 +111,6 @@ impl TraitEngine<'tcx> for FulfillmentContext<'tcx> {
code: FulfillmentErrorCode::CodeSelectionError(
SelectionError::Unimplemented,
),
points_at_arg_span: false,
// FIXME - does Chalk have a notation of 'root obligation'?
// This is just for diagnostics, so it's okay if this is wrong
root_obligation: obligation,
Expand All @@ -129,7 +127,6 @@ impl TraitEngine<'tcx> for FulfillmentContext<'tcx> {
code: FulfillmentErrorCode::CodeSelectionError(
SelectionError::Unimplemented,
),
points_at_arg_span: false,
// FIXME - does Chalk have a notation of 'root obligation'?
// This is just for diagnostics, so it's okay if this is wrong
root_obligation: obligation,
Expand Down
15 changes: 3 additions & 12 deletions compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ pub trait InferCtxtExt<'tcx> {
root_obligation: &PredicateObligation<'tcx>,
error: &SelectionError<'tcx>,
fallback_has_occurred: bool,
points_at_arg: bool,
);

/// Given some node representing a fn-like thing in the HIR map,
Expand Down Expand Up @@ -237,7 +236,6 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
root_obligation: &PredicateObligation<'tcx>,
error: &SelectionError<'tcx>,
fallback_has_occurred: bool,
points_at_arg: bool,
) {
let tcx = self.tcx;
let mut span = obligation.cause.span;
Expand Down Expand Up @@ -387,7 +385,6 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
&obligation,
&mut err,
&trait_ref,
points_at_arg,
have_alt_message,
) {
self.note_obligation_cause(&mut err, &obligation);
Expand Down Expand Up @@ -430,8 +427,8 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
err.span_label(enclosing_scope_span, s.as_str());
}

self.suggest_dereferences(&obligation, &mut err, trait_ref, points_at_arg);
self.suggest_fn_call(&obligation, &mut err, trait_ref, points_at_arg);
self.suggest_dereferences(&obligation, &mut err, trait_ref);
self.suggest_fn_call(&obligation, &mut err, trait_ref);
self.suggest_remove_reference(&obligation, &mut err, trait_ref);
self.suggest_semicolon_removal(&obligation, &mut err, span, trait_ref);
self.note_version_mismatch(&mut err, &trait_ref);
Expand Down Expand Up @@ -500,12 +497,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
// Changing mutability doesn't make a difference to whether we have
// an `Unsize` impl (Fixes ICE in #71036)
if !is_unsize {
self.suggest_change_mut(
&obligation,
&mut err,
trait_ref,
points_at_arg,
);
self.suggest_change_mut(&obligation, &mut err, trait_ref);
}

// If this error is due to `!: Trait` not implemented but `(): Trait` is
Expand Down Expand Up @@ -1214,7 +1206,6 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
&error.root_obligation,
selection_error,
fallback_has_occurred,
error.points_at_arg_span,
);
}
FulfillmentErrorCode::CodeProjectionError(ref e) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use crate::traits::normalize_projection_type;

use rustc_data_structures::fx::FxHashSet;
use rustc_data_structures::stack::ensure_sufficient_stack;
use rustc_data_structures::sync::Lrc;
use rustc_errors::{error_code, struct_span_err, Applicability, DiagnosticBuilder, Style};
use rustc_hir as hir;
use rustc_hir::def::DefKind;
Expand Down Expand Up @@ -54,7 +55,6 @@ pub trait InferCtxtExt<'tcx> {
obligation: &PredicateObligation<'tcx>,
err: &mut DiagnosticBuilder<'tcx>,
trait_ref: ty::PolyTraitRef<'tcx>,
points_at_arg: bool,
);

fn get_closure_name(
Expand All @@ -69,15 +69,13 @@ pub trait InferCtxtExt<'tcx> {
obligation: &PredicateObligation<'tcx>,
err: &mut DiagnosticBuilder<'_>,
trait_ref: ty::Binder<'tcx, ty::TraitRef<'tcx>>,
points_at_arg: bool,
);

fn suggest_add_reference_to_arg(
&self,
obligation: &PredicateObligation<'tcx>,
err: &mut DiagnosticBuilder<'_>,
trait_ref: &ty::Binder<'tcx, ty::TraitRef<'tcx>>,
points_at_arg: bool,
has_custom_message: bool,
) -> bool;

Expand All @@ -93,7 +91,6 @@ pub trait InferCtxtExt<'tcx> {
obligation: &PredicateObligation<'tcx>,
err: &mut DiagnosticBuilder<'_>,
trait_ref: ty::Binder<'tcx, ty::TraitRef<'tcx>>,
points_at_arg: bool,
);

fn suggest_semicolon_removal(
Expand Down Expand Up @@ -490,16 +487,19 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
obligation: &PredicateObligation<'tcx>,
err: &mut DiagnosticBuilder<'tcx>,
trait_ref: ty::PolyTraitRef<'tcx>,
points_at_arg: bool,
) {
// It only make sense when suggesting dereferences for arguments
if !points_at_arg {
let code = if let ObligationCauseCode::FunctionArgumentObligation { parent_code, .. } =
&obligation.cause.code
{
parent_code.clone()
} else {
return;
}
};
let param_env = obligation.param_env;
let body_id = obligation.cause.body_id;
let span = obligation.cause.span;
let real_trait_ref = match &obligation.cause.code {
let real_trait_ref = match &*code {
ObligationCauseCode::ImplDerivedObligation(cause)
| ObligationCauseCode::DerivedObligation(cause)
| ObligationCauseCode::BuiltinDerivedObligation(cause) => cause.parent_trait_ref,
Expand Down Expand Up @@ -584,7 +584,6 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
obligation: &PredicateObligation<'tcx>,
err: &mut DiagnosticBuilder<'_>,
trait_ref: ty::Binder<'tcx, ty::TraitRef<'tcx>>,
points_at_arg: bool,
) {
let self_ty = match trait_ref.self_ty().no_bound_vars() {
None => return,
Expand Down Expand Up @@ -656,11 +655,11 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
}
_ => return,
};
if points_at_arg {
if matches!(obligation.cause.code, ObligationCauseCode::FunctionArgumentObligation { .. }) {
// When the obligation error has been ensured to have been caused by
// an argument, the `obligation.cause.span` points at the expression
// of the argument, so we can provide a suggestion. This is signaled
// by `points_at_arg`. Otherwise, we give a more general note.
// of the argument, so we can provide a suggestion. Otherwise, we give
// a more general note.
err.span_suggestion_verbose(
obligation.cause.span.shrink_to_hi(),
&msg,
Expand All @@ -677,18 +676,21 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
obligation: &PredicateObligation<'tcx>,
err: &mut DiagnosticBuilder<'_>,
trait_ref: &ty::Binder<'tcx, ty::TraitRef<'tcx>>,
points_at_arg: bool,
has_custom_message: bool,
) -> bool {
let span = obligation.cause.span;
let points_at_for_iter = matches!(
span.ctxt().outer_expn_data().kind,
ExpnKind::Desugaring(DesugaringKind::ForLoop(ForLoopLoc::IntoIter))
);

if !points_at_arg && !points_at_for_iter {
let code = if let ObligationCauseCode::FunctionArgumentObligation { parent_code, .. } =
&obligation.cause.code
{
parent_code.clone()
} else if let ExpnKind::Desugaring(DesugaringKind::ForLoop(ForLoopLoc::IntoIter)) =
span.ctxt().outer_expn_data().kind
{
Lrc::new(obligation.cause.code.clone())
} else {
return false;
}
};

// List of traits for which it would be nonsensical to suggest borrowing.
// For instance, immutable references are always Copy, so suggesting to
Expand Down Expand Up @@ -787,7 +789,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
return false;
};

if let ObligationCauseCode::ImplDerivedObligation(obligation) = &obligation.cause.code {
if let ObligationCauseCode::ImplDerivedObligation(obligation) = &*code {
let expected_trait_ref = obligation.parent_trait_ref.skip_binder();
let new_imm_trait_ref =
ty::TraitRef::new(obligation.parent_trait_ref.def_id(), imm_substs);
Expand All @@ -799,7 +801,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
return try_borrowing(new_mut_trait_ref, expected_trait_ref, true, &[]);
}
} else if let ObligationCauseCode::BindingObligation(_, _)
| ObligationCauseCode::ItemObligation(_) = &obligation.cause.code
| ObligationCauseCode::ItemObligation(_) = &*code
{
if try_borrowing(
ty::TraitRef::new(trait_ref.def_id, imm_substs),
Expand Down Expand Up @@ -891,8 +893,12 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
obligation: &PredicateObligation<'tcx>,
err: &mut DiagnosticBuilder<'_>,
trait_ref: ty::Binder<'tcx, ty::TraitRef<'tcx>>,
points_at_arg: bool,
) {
let points_at_arg = matches!(
obligation.cause.code,
ObligationCauseCode::FunctionArgumentObligation { .. },
);

let span = obligation.cause.span;
if let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(span) {
let refs_number =
Expand Down Expand Up @@ -2289,6 +2295,56 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
)
});
}
ObligationCauseCode::FunctionArgumentObligation {
arg_hir_id,
call_hir_id,
ref parent_code,
} => {
let hir = self.tcx.hir();
if let Some(Node::Expr(expr @ hir::Expr { kind: hir::ExprKind::Block(..), .. })) =
hir.find(arg_hir_id)
{
let in_progress_typeck_results =
self.in_progress_typeck_results.map(|t| t.borrow());
let parent_id = hir.local_def_id(hir.get_parent_item(arg_hir_id));
let typeck_results: &TypeckResults<'tcx> = match &in_progress_typeck_results {
Some(t) if t.hir_owner == parent_id => t,
_ => self.tcx.typeck(parent_id),
};
let ty = typeck_results.expr_ty_adjusted(expr);
let span = expr.peel_blocks().span;
if Some(span) != err.span.primary_span() {
err.span_label(
span,
&if ty.references_error() {
String::new()
} else {
format!("this tail expression is of type `{:?}`", ty)
},
);
}
}
if let Some(Node::Expr(hir::Expr {
kind:
hir::ExprKind::Call(hir::Expr { span, .. }, _)
| hir::ExprKind::MethodCall(_, span, ..),
..
})) = hir.find(call_hir_id)
{
if Some(*span) != err.span.primary_span() {
err.span_label(*span, "required by a bound introduced by this call");
}
}
ensure_sufficient_stack(|| {
self.note_obligation_cause_code(
err,
predicate,
&parent_code,
obligated_types,
seen_requirements,
)
});
}
ObligationCauseCode::CompareImplMethodObligation {
item_name,
trait_item_def_id,
Expand Down
11 changes: 10 additions & 1 deletion compiler/rustc_typeck/src/check/callee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,16 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
arg_exprs: &'tcx [hir::Expr<'tcx>],
expected: Expectation<'tcx>,
) -> Ty<'tcx> {
let original_callee_ty = self.check_expr(callee_expr);
let original_callee_ty = match &callee_expr.kind {
hir::ExprKind::Path(hir::QPath::Resolved(..) | hir::QPath::TypeRelative(..)) => self
.check_expr_with_expectation_and_args(
callee_expr,
Expectation::NoExpectation,
arg_exprs,
),
_ => self.check_expr(callee_expr),
};

let expr_ty = self.structurally_resolved_type(call_expr.span, original_callee_ty);

let mut autoderef = self.autoderef(callee_expr.span, expr_ty);
Expand Down
8 changes: 1 addition & 7 deletions compiler/rustc_typeck/src/check/coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -707,13 +707,7 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {

// Object safety violations or miscellaneous.
Err(err) => {
self.report_selection_error(
obligation.clone(),
&obligation,
&err,
false,
false,
);
self.report_selection_error(obligation.clone(), &obligation, &err, false);
// Treat this like an obligation and follow through
// with the unsizing - the lack of a coercion should
// be silent, as it causes a type mismatch later.
Expand Down
Loading