Skip to content

Add some needed comments in adjust_fulfillment_errors.rs #115289

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 19, 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
55 changes: 27 additions & 28 deletions compiler/rustc_hir_typeck/src/fn_ctxt/adjust_fulfillment_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,27 +33,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
};

let generics = self.tcx.generics_of(def_id);
let predicate_args = match unsubstituted_pred.kind().skip_binder() {
ty::ClauseKind::Trait(pred) => pred.trait_ref.args.to_vec(),
ty::ClauseKind::Projection(pred) => pred.projection_ty.args.to_vec(),
ty::ClauseKind::ConstArgHasType(arg, ty) => {
vec![ty.into(), arg.into()]
}
ty::ClauseKind::ConstEvaluatable(e) => vec![e.into()],
_ => return false,
};
let (predicate_args, predicate_self_type_to_point_at) =
match unsubstituted_pred.kind().skip_binder() {
ty::ClauseKind::Trait(pred) => {
(pred.trait_ref.args.to_vec(), Some(pred.self_ty().into()))
}
ty::ClauseKind::Projection(pred) => (pred.projection_ty.args.to_vec(), None),
ty::ClauseKind::ConstArgHasType(arg, ty) => (vec![ty.into(), arg.into()], None),
ty::ClauseKind::ConstEvaluatable(e) => (vec![e.into()], None),
_ => return false,
};

let direct_param = if let ty::ClauseKind::Trait(pred) = unsubstituted_pred.kind().skip_binder()
&& let ty = pred.trait_ref.self_ty()
&& let ty::Param(_param) = ty.kind()
&& let Some(arg) = predicate_args.get(0)
&& let ty::GenericArgKind::Type(arg_ty) = arg.unpack()
&& arg_ty == ty
Comment on lines -48 to -51
Copy link
Member Author

Choose a reason for hiding this comment

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

These last four &&s are either redundant (by construction of predicate_args) or unneeded (checking if it's a param), so I just folded it into the match above.

{
Some(*arg)
} else {
None
};
let find_param_matching = |matches: &dyn Fn(ty::ParamTerm) -> bool| {
predicate_args.iter().find_map(|arg| {
arg.walk().find_map(|arg| {
Expand Down Expand Up @@ -112,18 +102,21 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let qpath =
if let hir::ExprKind::Path(qpath) = expr.kind { Some(qpath) } else { None };

(Some(*expr), qpath)
(Some(&expr.kind), qpath)
}
hir::Node::Ty(hir::Ty { kind: hir::TyKind::Path(qpath), .. }) => (None, Some(*qpath)),
_ => return false,
};

if let Some(qpath) = qpath {
if let Some(param) = direct_param {
if self.point_at_path_if_possible(error, def_id, param, &qpath) {
return true;
}
// Prefer pointing at the turbofished arg that corresponds to the
// self type of the failing predicate over anything else.
if let Some(param) = predicate_self_type_to_point_at
&& self.point_at_path_if_possible(error, def_id, param, &qpath)
{
return true;
}

if let hir::Node::Expr(hir::Expr {
kind: hir::ExprKind::Call(callee, args),
hir_id: call_hir_id,
Expand Down Expand Up @@ -166,11 +159,16 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}

match expr.map(|e| e.kind) {
match expr {
Some(hir::ExprKind::MethodCall(segment, receiver, args, ..)) => {
if let Some(param) = direct_param
if let Some(param) = predicate_self_type_to_point_at
&& self.point_at_generic_if_possible(error, def_id, param, segment)
{
// HACK: This is not correct, since `predicate_self_type_to_point_at` might
Copy link
Member Author

@compiler-errors compiler-errors Aug 27, 2023

Choose a reason for hiding this comment

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

This shouldn't matter in practice, but I think it's important to note, since it can lead to weird diagnostics depending on how the FunctionArgumentObligation is used.

// not actually correspond to the receiver of the method call. But we
// re-adjust the cause code here in order to prefer pointing at one of
// the method's turbofish segments but still use `FunctionArgumentObligation`
// elsewhere. Hopefully this doesn't break something.
error.obligation.cause.map_code(|parent_code| {
ObligationCauseCode::FunctionArgumentObligation {
arg_hir_id: receiver.hir_id,
Expand All @@ -180,6 +178,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
});
return true;
}

for param in [param_to_point_at, fallback_param_to_point_at, self_param_to_point_at]
.into_iter()
.flatten()
Expand Down Expand Up @@ -237,7 +236,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}

for param in [
direct_param,
predicate_self_type_to_point_at,
param_to_point_at,
fallback_param_to_point_at,
self_param_to_point_at,
Expand Down
11 changes: 7 additions & 4 deletions compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -318,13 +318,16 @@ impl<'a, 'tcx> AstConv<'tcx> for FnCtxt<'a, 'tcx> {
fn record_ty(&self, hir_id: hir::HirId, ty: Ty<'tcx>, span: Span) {
// FIXME: normalization and escaping regions
let ty = if !ty.has_escaping_bound_vars() {
if let ty::Alias(
ty::AliasKind::Projection | ty::AliasKind::Weak,
ty::AliasTy { args, def_id, .. },
) = ty.kind()
// NOTE: These obligations are 100% redundant and are implied by
Copy link
Member Author

Choose a reason for hiding this comment

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

Want to note that this can be removed in the future, since they're not here for correctness.

// WF obligations that are registered elsewhere, but they have a
// better cause code assigned to them in `add_required_obligations_for_hir`.
// This means that they should shadow obligations with worse spans.
if let ty::Alias(ty::Projection | ty::Weak, ty::AliasTy { args, def_id, .. }) =
ty.kind()
{
self.add_required_obligations_for_hir(span, *def_id, args, hir_id);
}

self.normalize(span, ty)
} else {
ty
Expand Down