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

Disallow nested impl traits #17541

Merged
merged 1 commit into from
Jul 5, 2024
Merged

Conversation

ShoyuVanilla
Copy link
Contributor

Fixes #17498

The above issue is due to formatting self referencing, recursive bound like Implemented(^0.0: TraitId(0)<[?0 := ^0.0]>) on the codes like;

trait Foo<T> {}

trait Bar {}

fn test(foo: impl Foo<impl Bar>) { ... }

When lowering predicate impl Foo<impl Bar> in trait_environment_query, the outer impl Foo<...> is treated as predicates, so the first TypeRef that passes the following code is impl Bar;

ImplTraitLoweringState::Variable(counter) => {
let idx = counter.get();
// Count the number of `impl Trait` things that appear within our bounds.
// Since t hose have been emitted as implicit type args already.
counter.set(idx + count_impl_traits(type_ref) as u16);
let (
_parent_params,
self_param,
type_params,
const_params,
_impl_trait_params,
lifetime_params,
) = self
.generics()
.expect("variable impl trait lowering must be in a generic def")
.provenance_split();
TyKind::BoundVar(BoundVar::new(
self.in_binders,
idx as usize
+ self_param as usize
+ type_params
+ const_params
+ lifetime_params,
))
.intern(Interner)

and thus the idx is 0 in the above context.

But the following code sets self_ty as the BoundVar with idx = 0 because the target param id for predicate impl Foo<...> is 0 since impl Foo is the first generic-like parameter of fn test;

pub(crate) fn lower_where_predicate<'b>(
&'b self,
where_predicate: &'b WherePredicate,
&def: &GenericDefId,
ignore_bindings: bool,
) -> impl Iterator<Item = QuantifiedWhereClause> + 'b {
match where_predicate {
WherePredicate::ForLifetime { target, bound, .. }
| WherePredicate::TypeBound { target, bound } => {
let self_ty = match target {
WherePredicateTypeTarget::TypeRef(type_ref) => self.lower_ty(type_ref),
&WherePredicateTypeTarget::TypeOrConstParam(local_id) => {
let param_id = hir_def::TypeOrConstParamId { parent: def, local_id };
match self.type_param_mode {
ParamLoweringMode::Placeholder => {
TyKind::Placeholder(to_placeholder_idx(self.db, param_id))
}
ParamLoweringMode::Variable => {
let idx = generics(self.db.upcast(), def)
.type_or_const_param_idx(param_id)
.expect("matching generics");
TyKind::BoundVar(BoundVar::new(DebruijnIndex::INNERMOST, idx))
}
}
.intern(Interner)
}
};
Either::Left(self.lower_type_bound(bound, self_ty, ignore_bindings))

For the codes like;

trait Foo {
    type Assoc;
}

trait Bar {}

fn test(foo: impl Foo<Assoc = impl Bar>) { ... }

similar recursive bound doesn't happen because the following codes "count the number of impl Trait things that appear before the target of our bound."

let ty = if let Some(target_param_idx) = target_param_idx {
let mut counter = 0;
let generics = self.generics().expect("generics in scope");
for (idx, data) in generics.iter_self_type_or_consts() {
// Count the number of `impl Trait` things that appear before
// the target of our `bound`.
// Our counter within `impl_trait_mode` should be that number
// to properly lower each types within `type_ref`
if data.type_param().is_some_and(|p| {
p.provenance == TypeParamProvenance::ArgumentImplTrait
}) {
counter += 1;
}
if idx == *target_param_idx {
break;
}
}
let mut ext = TyLoweringContext::new_maybe_unowned(
self.db,
self.resolver,
self.owner,
)
.with_type_param_mode(self.type_param_mode);
match &self.impl_trait_mode {
ImplTraitLoweringState::Param(_) => {
ext.impl_trait_mode =
ImplTraitLoweringState::Param(Cell::new(counter));
}
ImplTraitLoweringState::Variable(_) => {
ext.impl_trait_mode =
ImplTraitLoweringState::Variable(Cell::new(counter));
}

Instead of doing similar thing like nested impl Foo<impl Bar> thing, this PR lowers such nested impl traits into error types in the similar manner as the rustc does in the following lines (and of course, allows lowering and inferencing nested impl traits for the cases that rustc permits);

(Though rustc emits E0666😈, I skipped diagnostics since gathering diagnostics in hir-def has no conventions so far 😅)

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

Veykril commented Jul 5, 2024

Thanks!
@bors r+

@bors
Copy link
Collaborator

bors commented Jul 5, 2024

📌 Commit 4bb623d has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Jul 5, 2024

⌛ Testing commit 4bb623d with merge f2afcb8...

@bors
Copy link
Collaborator

bors commented Jul 5, 2024

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

@bors bors merged commit f2afcb8 into rust-lang:master Jul 5, 2024
11 checks passed
@ShoyuVanilla ShoyuVanilla deleted the nested-impl-traits branch July 5, 2024 11:16
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.

Stack overflow when calling "impl Fn(&impl Trait)"
4 participants