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

fix: Wrong Sized predicate for generic_predicates_for_param #17939

Merged
merged 1 commit into from
Aug 22, 2024

Conversation

ShoyuVanilla
Copy link
Member

I found this gathers wrong Self: Sized bound while implementing object safety, though I couldn't find proper test for this.

If we call generic_predicates_for_param to Bar in the following code;

trait Foo<T: ?Sized> {}
trait Bar<T: Foo<Self> + ?Sized> {}

it returns T: Sized and Self: Sized bound, because normaly, the ?Sized bound applied properly in L1059 with;

pub(crate) fn lower_type_bound(
&'a self,
bound: &'a Interned<TypeBound>,
self_ty: Ty,
ignore_bindings: bool,
) -> impl Iterator<Item = QuantifiedWhereClause> + 'a {
let mut trait_ref = None;
let clause = match bound.as_ref() {
TypeBound::Path(path, TraitBoundModifier::None) => {
trait_ref = self.lower_trait_ref_from_path(path, Some(self_ty));
trait_ref.clone().map(WhereClause::Implemented).map(crate::wrap_empty_binders)
}
TypeBound::Path(path, TraitBoundModifier::Maybe) => {
let sized_trait = self
.db
.lang_item(self.resolver.krate(), LangItem::Sized)
.and_then(|lang_item| lang_item.as_trait());
// Don't lower associated type bindings as the only possible relaxed trait bound
// `?Sized` has no of them.
// If we got another trait here ignore the bound completely.
let trait_id = self
.lower_trait_ref_from_path(path, Some(self_ty.clone()))
.map(|trait_ref| trait_ref.hir_trait_id());
if trait_id == sized_trait {
self.unsized_types.borrow_mut().insert(self_ty);
}
None

But we filter them before it is lowered with that function here;

// we have to filter out all other predicates *first*, before attempting to lower them
let predicate = |(pred, &def): &(&_, _)| match pred {
WherePredicate::ForLifetime { target, bound, .. }
| WherePredicate::TypeBound { target, bound, .. } => {
let invalid_target = match target {
WherePredicateTypeTarget::TypeRef(type_ref) => {
ctx.lower_ty_only_param(type_ref) != Some(param_id)
}
&WherePredicateTypeTarget::TypeOrConstParam(local_id) => {
let target_id = TypeOrConstParamId { parent: def, local_id };
target_id != param_id
}
};
if invalid_target {
return false;
}
match &**bound {
TypeBound::ForLifetime(_, path) | TypeBound::Path(path, _) => {
// Only lower the bound if the trait could possibly define the associated
// type we're looking for.
let Some(assoc_name) = &assoc_name else { return true };
let Some(TypeNs::TraitId(tr)) =
resolver.resolve_path_in_type_ns_fully(db.upcast(), path)
else {
return false;
};
all_super_traits(db.upcast(), tr).iter().any(|tr| {
db.trait_data(*tr).items.iter().any(|(name, item)| {
matches!(item, AssocItemId::TypeAliasId(_)) && name == assoc_name
})
})
}
TypeBound::Lifetime(_) | TypeBound::Error => false,
}
}
WherePredicate::Lifetime { .. } => false,
};
let mut predicates: Vec<_> = resolver
.where_predicates_in_scope()
.filter(predicate)
.flat_map(|(pred, def)| {
ctx.lower_where_predicate(pred, def, true).map(|p| make_binders(db, &generics, p))
})
.collect();

So, the ?Sized bounded params are not gathered into ctx.unsized_types and thus we are applying them implicit Sized bound here;

if let Some(implicitly_sized_predicates) = implicitly_sized_clauses(
db,
param_id.parent,
&explicitly_unsized_tys,
&subst,
&resolver,
) {
predicates.extend(
implicitly_sized_predicates
.map(|p| make_binders(db, &generics, crate::wrap_empty_binders(p))),
);
};

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 21, 2024
@Veykril
Copy link
Member

Veykril commented Aug 22, 2024

Thanks!
@bors r+

@bors
Copy link
Collaborator

bors commented Aug 22, 2024

📌 Commit 71080cf has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Aug 22, 2024

⌛ Testing commit 71080cf with merge 011e3bb...

@bors
Copy link
Collaborator

bors commented Aug 22, 2024

☀️ Test successful - checks-actions
Approved by: Veykril
Pushing 011e3bb to master...

@bors bors merged commit 011e3bb into rust-lang:master Aug 22, 2024
11 checks passed
@ShoyuVanilla ShoyuVanilla deleted the maybe-sized-fix branch August 22, 2024 08:56
bors added a commit that referenced this pull request Aug 23, 2024
fix: Wrong `Self: Sized` predicate for trait assoc items

Again while implementing object safety like #17939 😅

If we call `generic_predicates_query` on `fn foo` in the following code;
```
trait Foo {
    fn foo();
}
```
It returns implicit bound `Self: Sized`, even though `Self` is not appearing as a generic parameter inside angle brackets, but as a parent generic parameter, "trait self".

This PR prevent pushing "implicit" `Self: Sized` predicates in such cases
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants