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

Use the correct logic for nested impl trait in assoc types #121838

Merged
merged 2 commits into from
Mar 5, 2024

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Mar 1, 2024

Previously we accidentally continued with the TAIT visitor, which allowed more than we wanted to.

r? @compiler-errors

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 1, 2024
Copy link
Member

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

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

I'm still pretty confused about the structure here. Why does an ImplTraitInAssocTypeCollector wrap an OpaqueTypeCollector? Can we not just have some mode: CollectionMode that switches on the walking/projection/etc behavior? Or could we just completely separate these codepaths altogether? I feel like there's unnecessary complication here motivated by code deduplication.

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 1, 2024
@oli-obk
Copy link
Contributor Author

oli-obk commented Mar 1, 2024

Yea, I guess I could use a runtime switch instead

@oli-obk oli-obk force-pushed the impl_trait_in_assoc_tys_fix branch from 06ab841 to 8364a06 Compare March 4, 2024 11:26
@oli-obk
Copy link
Contributor Author

oli-obk commented Mar 4, 2024

much better

@rustbot review

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 4, 2024
@compiler-errors
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Mar 4, 2024

📌 Commit 8364a06 has been approved by compiler-errors

It is now in the queue for this repository.

@bors 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 Mar 4, 2024
jhpratt added a commit to jhpratt/rust that referenced this pull request Mar 5, 2024
…x, r=compiler-errors

Use the correct logic for nested impl trait in assoc types

Previously we accidentally continued with the TAIT visitor, which allowed more than we wanted to.

r? `@compiler-errors`
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 5, 2024
Rollup of 15 pull requests

Successful merges:

 - rust-lang#121065 (Add basic i18n guidance for `Display`)
 - rust-lang#121202 (Limit the number of names and values in check-cfg diagnostics)
 - rust-lang#121213 (Add an example to demonstrate how Rc::into_inner works)
 - rust-lang#121262 (Add vector time complexity)
 - rust-lang#121287 (Clarify/add `must_use` message for Rc/Arc/Weak::into_raw.)
 - rust-lang#121664 (Adjust error `yield`/`await` lowering)
 - rust-lang#121838 (Use the correct logic for nested impl trait in assoc types)
 - rust-lang#121860 (Add a tidy check that checks whether the fluent slugs only appear once)
 - rust-lang#121913 (Don't panic when waiting on poisoned queries)
 - rust-lang#121959 (Removing absolute path in proc-macro)
 - rust-lang#121975 (hir_analysis: enums return `None` in `find_field`)
 - rust-lang#121978 (Fix duplicated path in the "not found dylib" error)
 - rust-lang#121987 (pattern analysis: abort on arity mismatch)
 - rust-lang#121993 (Avoid using unnecessary queries when printing the query stack in panics)
 - rust-lang#121997 (interpret/cast: make more matches on FloatTy properly exhaustive)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 5, 2024
…x, r=compiler-errors

Use the correct logic for nested impl trait in assoc types

Previously we accidentally continued with the TAIT visitor, which allowed more than we wanted to.

r? ``@compiler-errors``
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 5, 2024
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#121213 (Add an example to demonstrate how Rc::into_inner works)
 - rust-lang#121262 (Add vector time complexity)
 - rust-lang#121287 (Clarify/add `must_use` message for Rc/Arc/Weak::into_raw.)
 - rust-lang#121664 (Adjust error `yield`/`await` lowering)
 - rust-lang#121826 (Use root obligation on E0277 for some cases)
 - rust-lang#121838 (Use the correct logic for nested impl trait in assoc types)
 - rust-lang#121913 (Don't panic when waiting on poisoned queries)
 - rust-lang#121987 (pattern analysis: abort on arity mismatch)
 - rust-lang#121993 (Avoid using unnecessary queries when printing the query stack in panics)
 - rust-lang#121997 (interpret/cast: make more matches on FloatTy properly exhaustive)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 20dde1e into rust-lang:master Mar 5, 2024
11 checks passed
@rustbot rustbot added this to the 1.78.0 milestone Mar 5, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 5, 2024
Rollup merge of rust-lang#121838 - oli-obk:impl_trait_in_assoc_tys_fix, r=compiler-errors

Use the correct logic for nested impl trait in assoc types

Previously we accidentally continued with the TAIT visitor, which allowed more than we wanted to.

r? ```@compiler-errors```
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 5, 2024
…_by_queries, r=compiler-errors

Merge impl_trait_in_assoc_types_defined_by query back into `opaque_types_defined_by`

Instead, when we're collecting opaques for associated items, we choose the right collection mode depending on whether we're collecting for an associated item of a trait impl or not.

r? `@compiler-errors`

follow up to rust-lang#121838
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 5, 2024
…_by_queries, r=compiler-errors

Merge impl_trait_in_assoc_types_defined_by query back into `opaque_types_defined_by`

Instead, when we're collecting opaques for associated items, we choose the right collection mode depending on whether we're collecting for an associated item of a trait impl or not.

r? ``@compiler-errors``

follow up to rust-lang#121838
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 5, 2024
…_by_queries, r=compiler-errors

Merge impl_trait_in_assoc_types_defined_by query back into `opaque_types_defined_by`

Instead, when we're collecting opaques for associated items, we choose the right collection mode depending on whether we're collecting for an associated item of a trait impl or not.

r? ```@compiler-errors```

follow up to rust-lang#121838
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 6, 2024
Rollup merge of rust-lang#121991 - oli-obk:merge_opaque_types_defined_by_queries, r=compiler-errors

Merge impl_trait_in_assoc_types_defined_by query back into `opaque_types_defined_by`

Instead, when we're collecting opaques for associated items, we choose the right collection mode depending on whether we're collecting for an associated item of a trait impl or not.

r? ```@compiler-errors```

follow up to rust-lang#121838
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants