Skip to content

Commit c958fab

Browse files
fmeasefmease
committed
Move distractingly lengthy error reporting code into new report_ambiguous_assoc_item
Moreover, in `probe_single_bound_for_assoc_item` unconditionally return an `Err(_)` if there's more than a single bound (aka ambiguity) instead of returning the first bound in some cases. This avoids triggering a debug assertion later on ("not enough bound vars"). See the added test for details.
1 parent a866e42 commit c958fab

5 files changed

Lines changed: 204 additions & 146 deletions

File tree

‎compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs‎

Lines changed: 136 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -403,6 +403,141 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
403403
})
404404
}
405405

406+
pub(super) fn report_ambiguous_assoc_item(
407+
&self,
408+
bound1: ty::PolyTraitRef<'tcx>,
409+
bound2: ty::PolyTraitRef<'tcx>,
410+
matching_candidates: impl Iterator<Item = ty::PolyTraitRef<'tcx>>,
411+
qself: AssocItemQSelf,
412+
assoc_tag: ty::AssocTag,
413+
assoc_ident: Ident,
414+
span: Span,
415+
constraint: Option<&hir::AssocItemConstraint<'tcx>>,
416+
) -> ErrorGuaranteed {
417+
let tcx = self.tcx();
418+
419+
let assoc_kind_str = assoc_tag_str(assoc_tag);
420+
let qself_str = qself.to_string(tcx);
421+
let mut err = self.dcx().create_err(crate::errors::AmbiguousAssocItem {
422+
span,
423+
assoc_kind: assoc_kind_str,
424+
assoc_ident,
425+
qself: &qself_str,
426+
});
427+
// Provide a more specific error code index entry for equality bindings.
428+
err.code(
429+
if let Some(constraint) = constraint
430+
&& let hir::AssocItemConstraintKind::Equality { .. } = constraint.kind
431+
{
432+
E0222
433+
} else {
434+
E0221
435+
},
436+
);
437+
438+
// FIXME(#97583): Print associated item bindings properly (i.e., not as equality
439+
// predicates!).
440+
// FIXME: Turn this into a structured, translatable & more actionable suggestion.
441+
let mut where_bounds = vec![];
442+
for bound in [bound1, bound2].into_iter().chain(matching_candidates) {
443+
let bound_id = bound.def_id();
444+
let assoc_item = tcx.associated_items(bound_id).find_by_ident_and_kind(
445+
tcx,
446+
assoc_ident,
447+
assoc_tag,
448+
bound_id,
449+
);
450+
let bound_span = assoc_item.and_then(|item| tcx.hir_span_if_local(item.def_id));
451+
452+
if let Some(bound_span) = bound_span {
453+
err.span_label(
454+
bound_span,
455+
format!("ambiguous `{assoc_ident}` from `{}`", bound.print_trait_sugared(),),
456+
);
457+
if let Some(constraint) = constraint {
458+
match constraint.kind {
459+
hir::AssocItemConstraintKind::Equality { term } => {
460+
let term: ty::Term<'_> = match term {
461+
hir::Term::Ty(ty) => self.lower_ty(ty).into(),
462+
hir::Term::Const(ct) => {
463+
let assoc_item =
464+
assoc_item.expect("assoc_item should be present");
465+
let projection_term = bound.map_bound(|trait_ref| {
466+
let item_segment = hir::PathSegment {
467+
ident: constraint.ident,
468+
hir_id: constraint.hir_id,
469+
res: Res::Err,
470+
args: Some(constraint.gen_args),
471+
infer_args: false,
472+
};
473+
474+
let alias_args = self.lower_generic_args_of_assoc_item(
475+
constraint.ident.span,
476+
assoc_item.def_id,
477+
&item_segment,
478+
trait_ref.args,
479+
);
480+
ty::AliasTerm::new_from_def_id(
481+
tcx,
482+
assoc_item.def_id,
483+
alias_args,
484+
)
485+
});
486+
487+
// FIXME(mgca): code duplication with other places we lower
488+
// the rhs' of associated const bindings
489+
let ty = projection_term.map_bound(|alias| {
490+
tcx.type_of(alias.def_id())
491+
.instantiate(tcx, alias.args)
492+
.skip_norm_wip()
493+
});
494+
let ty = super::bounds::check_assoc_const_binding_type(
495+
self,
496+
constraint.ident,
497+
ty,
498+
constraint.hir_id,
499+
);
500+
501+
self.lower_const_arg(ct, ty).into()
502+
}
503+
};
504+
if term.references_error() {
505+
continue;
506+
}
507+
// FIXME(#97583): This isn't syntactically well-formed!
508+
where_bounds.push(format!(
509+
" T: {trait}::{assoc_ident} = {term}",
510+
trait = bound.print_only_trait_path(),
511+
));
512+
}
513+
// FIXME: Provide a suggestion.
514+
hir::AssocItemConstraintKind::Bound { bounds: _ } => {}
515+
}
516+
} else {
517+
err.span_suggestion_verbose(
518+
span.with_hi(assoc_ident.span.lo()),
519+
"use fully-qualified syntax to disambiguate",
520+
format!("<{qself_str} as {}>::", bound.print_only_trait_path()),
521+
Applicability::MaybeIncorrect,
522+
);
523+
}
524+
} else {
525+
let trait_ = tcx.short_string(bound.print_only_trait_path(), err.long_ty_path());
526+
err.note(format!(
527+
"associated {assoc_kind_str} `{assoc_ident}` could derive from `{trait_}`",
528+
));
529+
}
530+
}
531+
if !where_bounds.is_empty() {
532+
err.help(format!(
533+
"consider introducing a new type parameter `T` and adding `where` constraints:\
534+
\n where\n T: {qself_str},\n{}",
535+
where_bounds.join(",\n"),
536+
));
537+
}
538+
err.emit()
539+
}
540+
406541
pub(crate) fn report_missing_self_ty_for_resolved_path(
407542
&self,
408543
trait_def_id: DefId,
@@ -568,7 +703,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
568703
}
569704
}
570705

571-
pub(super) fn report_ambiguous_assoc_item_path(
706+
fn report_ambiguous_assoc_item_path(
572707
&self,
573708
span: Span,
574709
types: &[String],

‎compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs‎

Lines changed: 11 additions & 130 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ use rustc_infer::infer::{InferCtxt, TyCtxtInferExt};
3636
use rustc_infer::traits::DynCompatibilityViolation;
3737
use rustc_macros::{TypeFoldable, TypeVisitable};
3838
use rustc_middle::middle::stability::AllowUnstable;
39-
use rustc_middle::ty::print::PrintPolyTraitRefExt as _;
4039
use rustc_middle::ty::{
4140
self, Const, FnSigKind, GenericArgKind, GenericArgsRef, GenericParamDefKind, LitToConstInput,
4241
Ty, TyCtxt, TypeSuperFoldable, TypeVisitableExt, TypingMode, Unnormalized, Upcast,
@@ -1281,13 +1280,11 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
12811280
where
12821281
I: Iterator<Item = ty::PolyTraitRef<'tcx>>,
12831282
{
1284-
let tcx = self.tcx();
1285-
12861283
let mut matching_candidates = all_candidates().filter(|r| {
12871284
self.probe_trait_that_defines_assoc_item(r.def_id(), assoc_tag, assoc_ident)
12881285
});
12891286

1290-
let Some(bound) = matching_candidates.next() else {
1287+
let Some(bound1) = matching_candidates.next() else {
12911288
return Err(self.report_unresolved_assoc_item(
12921289
all_candidates,
12931290
qself,
@@ -1297,137 +1294,21 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
12971294
constraint,
12981295
));
12991296
};
1300-
debug!(?bound);
13011297

13021298
if let Some(bound2) = matching_candidates.next() {
1303-
debug!(?bound2);
1304-
1305-
let assoc_kind_str = errors::assoc_tag_str(assoc_tag);
1306-
let qself_str = qself.to_string(tcx);
1307-
let mut err = self.dcx().create_err(crate::errors::AmbiguousAssocItem {
1308-
span,
1309-
assoc_kind: assoc_kind_str,
1299+
return Err(self.report_ambiguous_assoc_item(
1300+
bound1,
1301+
bound2,
1302+
matching_candidates,
1303+
qself,
1304+
assoc_tag,
13101305
assoc_ident,
1311-
qself: &qself_str,
1312-
});
1313-
// Provide a more specific error code index entry for equality bindings.
1314-
err.code(
1315-
if let Some(constraint) = constraint
1316-
&& let hir::AssocItemConstraintKind::Equality { .. } = constraint.kind
1317-
{
1318-
E0222
1319-
} else {
1320-
E0221
1321-
},
1322-
);
1323-
1324-
// FIXME(#97583): Print associated item bindings properly (i.e., not as equality
1325-
// predicates!).
1326-
// FIXME: Turn this into a structured, translatable & more actionable suggestion.
1327-
let mut where_bounds = vec![];
1328-
for bound in [bound, bound2].into_iter().chain(matching_candidates) {
1329-
let bound_id = bound.def_id();
1330-
let assoc_item = tcx.associated_items(bound_id).find_by_ident_and_kind(
1331-
tcx,
1332-
assoc_ident,
1333-
assoc_tag,
1334-
bound_id,
1335-
);
1336-
let bound_span = assoc_item.and_then(|item| tcx.hir_span_if_local(item.def_id));
1337-
1338-
if let Some(bound_span) = bound_span {
1339-
err.span_label(
1340-
bound_span,
1341-
format!("ambiguous `{assoc_ident}` from `{}`", bound.print_trait_sugared(),),
1342-
);
1343-
if let Some(constraint) = constraint {
1344-
match constraint.kind {
1345-
hir::AssocItemConstraintKind::Equality { term } => {
1346-
let term: ty::Term<'_> = match term {
1347-
hir::Term::Ty(ty) => self.lower_ty(ty).into(),
1348-
hir::Term::Const(ct) => {
1349-
let assoc_item =
1350-
assoc_item.expect("assoc_item should be present");
1351-
let projection_term = bound.map_bound(|trait_ref| {
1352-
let item_segment = hir::PathSegment {
1353-
ident: constraint.ident,
1354-
hir_id: constraint.hir_id,
1355-
res: Res::Err,
1356-
args: Some(constraint.gen_args),
1357-
infer_args: false,
1358-
};
1359-
1360-
let alias_args = self.lower_generic_args_of_assoc_item(
1361-
constraint.ident.span,
1362-
assoc_item.def_id,
1363-
&item_segment,
1364-
trait_ref.args,
1365-
);
1366-
ty::AliasTerm::new_from_def_id(
1367-
tcx,
1368-
assoc_item.def_id,
1369-
alias_args,
1370-
)
1371-
});
1372-
1373-
// FIXME(mgca): code duplication with other places we lower
1374-
// the rhs' of associated const bindings
1375-
let ty = projection_term.map_bound(|alias| {
1376-
tcx.type_of(alias.def_id())
1377-
.instantiate(tcx, alias.args)
1378-
.skip_norm_wip()
1379-
});
1380-
let ty = bounds::check_assoc_const_binding_type(
1381-
self,
1382-
constraint.ident,
1383-
ty,
1384-
constraint.hir_id,
1385-
);
1386-
1387-
self.lower_const_arg(ct, ty).into()
1388-
}
1389-
};
1390-
if term.references_error() {
1391-
continue;
1392-
}
1393-
// FIXME(#97583): This isn't syntactically well-formed!
1394-
where_bounds.push(format!(
1395-
" T: {trait}::{assoc_ident} = {term}",
1396-
trait = bound.print_only_trait_path(),
1397-
));
1398-
}
1399-
// FIXME: Provide a suggestion.
1400-
hir::AssocItemConstraintKind::Bound { bounds: _ } => {}
1401-
}
1402-
} else {
1403-
err.span_suggestion_verbose(
1404-
span.with_hi(assoc_ident.span.lo()),
1405-
"use fully-qualified syntax to disambiguate",
1406-
format!("<{qself_str} as {}>::", bound.print_only_trait_path()),
1407-
Applicability::MaybeIncorrect,
1408-
);
1409-
}
1410-
} else {
1411-
let trait_ =
1412-
tcx.short_string(bound.print_only_trait_path(), err.long_ty_path());
1413-
err.note(format!(
1414-
"associated {assoc_kind_str} `{assoc_ident}` could derive from `{trait_}`",
1415-
));
1416-
}
1417-
}
1418-
if !where_bounds.is_empty() {
1419-
err.help(format!(
1420-
"consider introducing a new type parameter `T` and adding `where` constraints:\
1421-
\n where\n T: {qself_str},\n{}",
1422-
where_bounds.join(",\n"),
1423-
));
1424-
let reported = err.emit();
1425-
return Err(reported);
1426-
}
1427-
err.emit();
1306+
span,
1307+
constraint,
1308+
));
14281309
}
14291310

1430-
Ok(bound)
1311+
Ok(bound1)
14311312
}
14321313

14331314
/// Lower a [type-relative](hir::QPath::TypeRelative) path in type position to a type.

‎tests/crashes/139387.rs‎

Lines changed: 0 additions & 15 deletions
This file was deleted.

‎tests/ui/associated-type-bounds/return-type-notation/path-ambiguous-late-bound-vars.rs‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// We used to lower the ambiguous `T::f(..)` to `<T as B>::f::{type#0}` after emitting the error.
2+
// Meaning we picked one of the candidates and proceeded instead of bailing out early.
3+
// However, sensibly RBV doesn't register any bound vars for ambiguous RTN[^1], so later on when
4+
// wrapping the predicate (here: WellFormed) into a Binder we would correctly fail bound var
5+
// validation (in debug mode).
6+
//
7+
// We now bail out early and thus prevent nonsensical types from getting leaked to subsequent
8+
// compiler passes.
9+
//
10+
// [^1]: It actually maintains its own bespoke lowering function for type-relative paths that
11+
// mirrors the one in HIR ty lowering.
12+
13+
// issue: <https://github.com/rust-lang/rust/issues/139387>
14+
//@ needs-rustc-debug-assertions
15+
#![feature(return_type_notation)]
16+
17+
trait A {
18+
fn f() -> impl Sized;
19+
}
20+
21+
trait B {
22+
fn f<'b>() -> impl Sized;
23+
}
24+
25+
fn f<T: A + B>()
26+
where
27+
T::f(..):, //~ ERROR ambiguous associated function
28+
{
29+
}
30+
31+
fn main() {}

‎tests/ui/associated-type-bounds/return-type-notation/path-ambiguous-late-bound-vars.stderr‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
error[E0221]: ambiguous associated function `f` in bounds of `T`
2+
--> $DIR/path-ambiguous-late-bound-vars.rs:27:5
3+
|
4+
LL | fn f() -> impl Sized;
5+
| --------------------- ambiguous `f` from `A`
6+
...
7+
LL | fn f<'b>() -> impl Sized;
8+
| ------------------------- ambiguous `f` from `B`
9+
...
10+
LL | T::f(..):,
11+
| ^^^^^^^^ ambiguous associated function `f`
12+
|
13+
help: use fully-qualified syntax to disambiguate
14+
|
15+
LL - T::f(..):,
16+
LL + <T as B>::f(..):,
17+
|
18+
help: use fully-qualified syntax to disambiguate
19+
|
20+
LL - T::f(..):,
21+
LL + <T as A>::f(..):,
22+
|
23+
24+
error: aborting due to 1 previous error
25+
26+
For more information about this error, try `rustc --explain E0221`.

0 commit comments

Comments
 (0)