Skip to content

feat: trait associated type bounds#8594

Merged
jfecher merged 19 commits intomasterfrom
ab/trait-associated-type-bounds
May 21, 2025
Merged

feat: trait associated type bounds#8594
jfecher merged 19 commits intomasterfrom
ab/trait-associated-type-bounds

Conversation

@asterite
Copy link
Copy Markdown
Collaborator

@asterite asterite commented May 20, 2025

Description

Problem

A requirement for #8395

Summary

In order to implement this several things needed to be done:

  1. First, of course, introduce the new syntax
  2. Then, we need to resolve and store the bounds on trait associated types. This is stored in the associated_type_bounds field of Trait.
  3. We also need to verify that associated types on trait impls match the above bounds.
  4. Then, if we have a where clause like T: Foo where Foo has an associated type E with a bound on it, we need to introduce those bounds on the associated type. However, I faced a challenge here because traits were collected after defining function metas, and it's when collecting traits that we find out about these bounds. For this, I split collect_traits into two: collect_traits and collect_trait_methods. collect_traits runs before defining function metas and it's here that we collect these bounds. This worked!
  5. Another challenge was that after these bounds were turned into trait constraints, in trait impls they would give an error saying that these constraints didn't match the parent trait. For example in this code:
    trait Foo {
        type E: Bar;
        fn bar(self) -> Self::E;
    }
    
    trait Bar {
        fn bar_method(self);
    }
    
    impl<T> Eq for Baz<T> where T: Foo {
        fn eq(self, other: Self) -> bool {}
    }
    We need the constraint on Foo::E to be Bar inside eq, but for that to work it needed to be put in the trait_constraints field of FuncMeta. But if we do that then those constraints don't match the one (none) on the eq definition of the Eq trait.
    For this, I decided to split the trait_constraints field of FuncMeta into two:
    • trait_constraints: these are just the ones from the where clause on the function
    • extra_trait_constraints: these are the ones from the parent trait impl, and also ones for implicitly added named generics (like Foo:E above)
      Then when checking that the trait constraints match, we just check in trait_constraints. For knowing all the trait constraints needed for the function to work, there's a new all_trait_constraints() method.
  6. Finally, to really prove that this works as expected, I changed BuildHasher from being BuildHasher<H> to having H as an associated type with a bound. It worked!

The one thing missing (there's a TODO about this) is being able to put a where clause on a trait, with that where clause referring to a trait that has an associated type with bounds. This example in particular:

trait Foo {
    type E: Bar;
    fn bar(self) -> Self::E;
}

trait Bar {
    fn bar_method(self);
}

trait Baz<T>
where
    T: Foo,
{
    fn baz(t: T) {
        let bar = t.bar();
        bar.bar_method(); // This method here is not found
    }
}

Given that we currently don't have code for this, and it's not needed for BuildHasher, I'd prefer to implement it as a follow-up PR. A Trait object has a where_clause, but we'd likely need to split that into two too (not sure!).

Additional Context

Documentation*

Check one:

  • No documentation needed.
  • Documentation included in this PR.
  • [For Experimental Features] Documentation to be submitted in a separate PR.

PR Checklist*

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

@asterite asterite requested a review from a team May 21, 2025 15:32
Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Compilation Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: 590361d Previous: 4ee2d12 Ratio
rollup-base-private 21.82 s 15.94 s 1.37

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

Copy link
Copy Markdown
Member

@TomAFrench TomAFrench left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Contributor

@jfecher jfecher left a comment

Choose a reason for hiding this comment

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

LGTM

@jfecher jfecher added this pull request to the merge queue May 21, 2025
Merged via the queue into master with commit b2bfdb7 May 21, 2025
118 checks passed
@jfecher jfecher deleted the ab/trait-associated-type-bounds branch May 21, 2025 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants