Skip to content

Commit

Permalink
Auto merge of rust-lang#13264 - lowr:patch/no-dyn-without-trait, r=Ve…
Browse files Browse the repository at this point in the history
…ykril

Ensure at least one trait bound in `TyKind::DynTy`

One would expect `TyKind::DynTy` to have at least one trait bound, but we may produce a dyn type with no trait bounds at all. This patch prevents it by returning `TyKind::Error` in such cases.

An "empty" dyn type would have caused panic during method resolution without rust-lang#13257. Although already fixed, I think an invariant to never produce such types would help prevent similar problems in the future.
  • Loading branch information
bors committed Sep 19, 2022
2 parents 4d989b5 + 9845e37 commit bde76b9
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 5 deletions.
2 changes: 2 additions & 0 deletions crates/hir-ty/src/chalk_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,8 @@ impl TyExt for Ty {
let trait_ref = match self.kind(Interner) {
// The principal trait bound should be the first element of the bounds. This is an
// invariant ensured by `TyLoweringContext::lower_dyn_trait()`.
// FIXME: dyn types may not have principal trait and we don't want to return auto trait
// here.
TyKind::Dyn(dyn_ty) => dyn_ty.bounds.skip_binders().interned().get(0).and_then(|b| {
match b.skip_binders() {
WhereClause::Implemented(trait_ref) => Some(trait_ref),
Expand Down
18 changes: 13 additions & 5 deletions crates/hir-ty/src/lower.rs
Original file line number Diff line number Diff line change
Expand Up @@ -981,10 +981,11 @@ impl<'a> TyLoweringContext<'a> {

fn lower_dyn_trait(&self, bounds: &[Interned<TypeBound>]) -> Ty {
let self_ty = TyKind::BoundVar(BoundVar::new(DebruijnIndex::INNERMOST, 0)).intern(Interner);
// INVARIANT: The principal trait bound must come first. Others may be in any order but
// should be in the same order for the same set but possibly different order of bounds in
// the input.
// This invariant is used by `TyExt::dyn_trait()` and chalk.
// INVARIANT: The principal trait bound, if present, must come first. Others may be in any
// order but should be in the same order for the same set but possibly different order of
// bounds in the input.
// INVARIANT: If this function returns `DynTy`, there should be at least one trait bound.
// These invariants are utilized by `TyExt::dyn_trait()` and chalk.
let bounds = self.with_shifted_in(DebruijnIndex::ONE, |ctx| {
let mut bounds: Vec<_> = bounds
.iter()
Expand Down Expand Up @@ -1035,6 +1036,12 @@ impl<'a> TyLoweringContext<'a> {
return None;
}

if bounds.first().and_then(|b| b.trait_id()).is_none() {
// When there's no trait bound, that's an error. This happens when the trait refs
// are unresolved.
return None;
}

// As multiple occurrences of the same auto traits *are* permitted, we dedulicate the
// bounds. We shouldn't have repeated elements besides auto traits at this point.
bounds.dedup();
Expand All @@ -1046,7 +1053,8 @@ impl<'a> TyLoweringContext<'a> {
let bounds = crate::make_single_type_binders(bounds);
TyKind::Dyn(DynTy { bounds, lifetime: static_lifetime() }).intern(Interner)
} else {
// FIXME: report error (additional non-auto traits or associated type rebound)
// FIXME: report error
// (additional non-auto traits, associated type rebound, or no resolved trait)
TyKind::Error.intern(Interner)
}
}
Expand Down
12 changes: 12 additions & 0 deletions crates/hir-ty/src/tests/regression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1691,3 +1691,15 @@ fn macrostmts() -> u8 {
"#,
);
}

#[test]
fn dyn_with_unresolved_trait() {
check_types(
r#"
fn foo(a: &dyn DoesNotExist) {
a.bar();
//^&{unknown}
}
"#,
);
}

0 comments on commit bde76b9

Please sign in to comment.