Allow Trait inheritance with cycles on associated types #79209

spastorino · 2020-11-19T17:52:48Z

nikomatsakis

I thought I had some good creative ideas for tests but as I look at it, most of them are kind of the same as the tests you already have.

I guess I think we should test some scenarios that ARE still ambiguous, e.g.

trait Foo { type Item; }
trait Bar<T> { type Item; }
trait Baz: Foo + Bar<Self::Item> { }

src/test/ui/associated-type-bounds/super-trait-referencing.rs

compiler/rustc_typeck/src/astconv/mod.rs

compiler/rustc_typeck/src/check/fn_ctxt/mod.rs

compiler/rustc_typeck/src/collect.rs

compiler/rustc_typeck/src/astconv/mod.rs

compiler/rustc_typeck/src/collect.rs

nikomatsakis · 2020-11-19T18:33:02Z

compiler/rustc_typeck/src/collect.rs

+/// Ensures that the super-predicates of the trait with a `DefId`
+/// of `trait_def_id` are converted and stored. This also ensures that
+/// the transitive super-predicates are converted.
+fn super_predicates_that_define_assoc_type(


Hmm so this is a query... it's a bit unusual in that most query providers either operate on local def-ids or external def-ids but not both. (The typical strategy, in other words, would be to define this query by encoding it into the metadata.) But that seems kind of silly here, I don't see a reason to encode more into the metadata. So this setup is probably ok. But I feel like we have to be sure to setup the provider also for external crates or we will see ICEs -- I'm a bit surprised our tests aren't encountering them yet, actually.

bors · 2020-11-23T19:26:51Z

☔ The latest upstream changes (presumably #79345) made this pull request unmergeable. Please resolve the merge conflicts.

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

nikomatsakis

Can you add a test like

trait Foo<T> { }
trait Bar { type A; type B; }
trait Baz: Bar<B = u32> + Foo<Self::A> { }

I want to be sure that we correctly handle predicates like Bar<B = u32>, which can also define an assoc type (A, in this case)

compiler/rustc_typeck/src/astconv/mod.rs

compiler/rustc_infer/src/traits/util.rs

compiler/rustc_middle/src/query/mod.rs

compiler/rustc_typeck/src/collect.rs

compiler/rustc_middle/src/ty/context.rs

nikomatsakis · 2020-11-25T22:24:38Z

@bors r+

bors · 2020-11-25T22:24:40Z

📌 Commit 84f6b861433e61ab0de27980a9b9266ca4fff1ac has been approved by nikomatsakis

nikomatsakis · 2020-11-25T22:24:48Z

@bors rollup=never

bors · 2020-11-25T23:42:45Z

⌛ Testing commit 84f6b861433e61ab0de27980a9b9266ca4fff1ac with merge 4c9eee42b3766d927e66c92c378b3d55a7f3023f...

bors · 2020-11-25T23:49:36Z

💔 Test failed - checks-actions

spastorino · 2020-11-26T19:48:00Z

@bors retry

…rent platforms

nikomatsakis · 2020-11-28T14:51:06Z

@bors r+

bors · 2020-11-28T14:51:07Z

📌 Commit ada7c1f has been approved by nikomatsakis

bors · 2020-11-28T17:24:20Z

⌛ Testing commit ada7c1f with merge 60e4bbf0e17aa931e8b32579c31da2024d5e9890...

bors · 2020-11-28T21:39:20Z

💥 Test timed out

spastorino · 2020-11-29T20:30:52Z

@bors retry

bors · 2020-11-29T21:04:29Z

⌛ Testing commit ada7c1f with merge 349b3b3...

bors · 2020-11-29T23:14:31Z

☀️ Test successful - checks-actions
Approved by: nikomatsakis
Pushing 349b3b3 to master...

…self, r=nikomatsakis" This reverts commit 349b3b3, reversing changes made to b776d1c.

…elf, r=Mark-Simulacrum Revert "Auto merge of rust-lang#79209 r? `@nikomatsakis` This has caused some issues (rust-lang#79560) so better to revert and try to come up with a proper fix without rush.

rylev · 2020-12-03T11:08:35Z

@spastorino FYI: there was a minor perf regression from this PR. I'm still getting used to telling whether something is noise, but a 2% change on packed-simd seems like it's probably an actual regression (albeit a small one).
//cc @Mark-Simulacrum

edit: just noticed this was reverted in #79637

… r=nikomatsakis Allow Trait inheritance with cycles on associated types take 2 This reverts the revert of rust-lang#79209 and fixes the ICEs that's occasioned by that PR exposing some problems that are addressed in rust-lang#80648 and rust-lang#79811. For easier review I'd say, check only the last commit, the first one is just a revert of the revert of rust-lang#79209 which was already approved. This also could be considered part or the actual fix of rust-lang#79560 but I guess for that to be closed and fixed completely we would need to land rust-lang#80648 and rust-lang#79811 too. r? `@nikomatsakis` cc `@Aaron1011`

rust-highfive assigned nikomatsakis Nov 19, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 19, 2020

spastorino force-pushed the trait-inheritance-self branch from d6983e5 to 75a1a97 Compare November 19, 2020 17:55

nikomatsakis reviewed Nov 19, 2020

View reviewed changes

spastorino force-pushed the trait-inheritance-self branch 4 times, most recently from 7ce0390 to 3be4aaf Compare November 23, 2020 15:42

spastorino force-pushed the trait-inheritance-self branch 2 times, most recently from 34c5526 to 1401166 Compare November 23, 2020 19:48

nikomatsakis reviewed Nov 24, 2020

View reviewed changes

spastorino force-pushed the trait-inheritance-self branch from dff76fa to 50d7360 Compare November 25, 2020 13:35

nikomatsakis reviewed Nov 25, 2020

View reviewed changes

compiler/rustc_typeck/src/collect.rs Outdated Show resolved Hide resolved

spastorino force-pushed the trait-inheritance-self branch from 74dfa1e to 8ac093a Compare November 25, 2020 16:24

nikomatsakis requested changes Nov 25, 2020

View reviewed changes

compiler/rustc_middle/src/ty/context.rs Outdated Show resolved Hide resolved

spastorino force-pushed the trait-inheritance-self branch from 8ac093a to 84f6b86 Compare November 25, 2020 22:12

nikomatsakis approved these changes Nov 25, 2020

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 25, 2020

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 25, 2020

bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 26, 2020

bors added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 27, 2020

Return FxIndexSet instead of FxHashSet to avoid order errors on diffe…

ada7c1f

…rent platforms

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 28, 2020

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 28, 2020

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 29, 2020

bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 29, 2020

bors merged commit 349b3b3 into rust-lang:master Nov 29, 2020

rustbot added this to the 1.50.0 milestone Nov 29, 2020

hameerabbasi mentioned this pull request Nov 30, 2020

Internal Compiler Error while compiling diesel #79560

Closed

spastorino added a commit to spastorino/rust that referenced this pull request Dec 2, 2020

Revert "Auto merge of rust-lang#79209 - spastorino:trait-inheritance-…

37354eb

…self, r=nikomatsakis" This reverts commit 349b3b3, reversing changes made to b776d1c.

camelid added A-associated-items Area: Associated items (types, constants & functions) A-trait-system Area: Trait system labels Dec 3, 2020

estebank mentioned this pull request Dec 18, 2020

Confusing diagnostic for supertrait cycle #80164

Open

spastorino mentioned this pull request Jan 5, 2021

Allow Trait inheritance with cycles on associated types take 2 #80732

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow Trait inheritance with cycles on associated types #79209

Allow Trait inheritance with cycles on associated types #79209

spastorino commented Nov 19, 2020 •

edited

Loading

nikomatsakis left a comment

nikomatsakis Nov 19, 2020

bors commented Nov 23, 2020

nikomatsakis left a comment

nikomatsakis commented Nov 25, 2020

bors commented Nov 25, 2020

nikomatsakis commented Nov 25, 2020

bors commented Nov 25, 2020

bors commented Nov 25, 2020

spastorino commented Nov 26, 2020

nikomatsakis commented Nov 28, 2020

bors commented Nov 28, 2020

bors commented Nov 28, 2020

bors commented Nov 28, 2020

spastorino commented Nov 29, 2020

bors commented Nov 29, 2020

bors commented Nov 29, 2020

rylev commented Dec 3, 2020 •

edited

Loading

Allow Trait inheritance with cycles on associated types #79209

Allow Trait inheritance with cycles on associated types #79209

Conversation

spastorino commented Nov 19, 2020 • edited Loading

nikomatsakis left a comment

Choose a reason for hiding this comment

nikomatsakis Nov 19, 2020

Choose a reason for hiding this comment

bors commented Nov 23, 2020

nikomatsakis left a comment

Choose a reason for hiding this comment

nikomatsakis commented Nov 25, 2020

bors commented Nov 25, 2020

nikomatsakis commented Nov 25, 2020

bors commented Nov 25, 2020

bors commented Nov 25, 2020

spastorino commented Nov 26, 2020

nikomatsakis commented Nov 28, 2020

bors commented Nov 28, 2020

bors commented Nov 28, 2020

bors commented Nov 28, 2020

spastorino commented Nov 29, 2020

bors commented Nov 29, 2020

bors commented Nov 29, 2020

rylev commented Dec 3, 2020 • edited Loading

spastorino commented Nov 19, 2020 •

edited

Loading

rylev commented Dec 3, 2020 •

edited

Loading