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

Remove generic_associated_types_extended feature gate #133768

Merged
merged 1 commit into from
Dec 4, 2024

Conversation

compiler-errors
Copy link
Member

This PR retires nightly support for the generic_associated_types_extended feature. This feature hasn't received much attention in the last two years or so, and I believe the feature still remains both unsound and ICEy to use. I think that if we were to redesign and reimplement it, we'd want to first figure out how to implement it soundly, but in the mean time I'd prefer to clean this up.

r? @lcnr cc @jackh726 who added this feature gate I think

@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 Dec 2, 2024
Copy link
Member

@jackh726 jackh726 left a comment

Choose a reason for hiding this comment

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

This is fine by me - this was really only meant for experimentation, but I never had much time to dig into this.

However, I do think that there should be an open issue to track the general pattern here that we want to solve (there may be only), and that at least some of the tests modified here should reference that with known-bug.

@compiler-errors
Copy link
Member Author

Sure, I can open an issue about having GATs in dyn-compatible traits.

@jackh726
Copy link
Member

jackh726 commented Dec 2, 2024

Well, I guess that's one issue. But there is also the error in tests/ui/generic-associated-types/extended/lending_iterator.rs where we think the impl has stricter requirements than the trait. (There is also the more general implied bounds issue, but I don't think any of these tests are directly related to that.)

@lcnr
Copy link
Contributor

lcnr commented Dec 2, 2024

@bors r=lcnr,jackh726 rollup

@bors
Copy link
Contributor

bors commented Dec 2, 2024

📌 Commit 4eed6f6 has been approved by lcnr,jackh726

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 Dec 2, 2024
@compiler-errors
Copy link
Member Author

compiler-errors commented Dec 3, 2024

@bors r-

Lemme add those known-bug test annotations first

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 3, 2024
@compiler-errors
Copy link
Member Author

Added a known-bug for the subtle impl/trait compatibility bug. I don't think a tracking issue for object-safe GATs is super necessary, though I guess if you disagree feel free to open one.

@compiler-errors
Copy link
Member Author

@bors r=lcnr,jackh726

@bors
Copy link
Contributor

bors commented Dec 3, 2024

📌 Commit f91fd0c has been approved by lcnr,jackh726

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 3, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 3, 2024
Remove `generic_associated_types_extended` feature gate

This PR retires nightly support for the `generic_associated_types_extended` feature. This feature hasn't received much attention in the last two years or so, and I believe the feature still remains both unsound and ICEy to use. I think that if we were to redesign and reimplement it, we'd want to first figure out how to implement it soundly, but in the mean time I'd prefer to clean this up.

r? `@lcnr` cc `@jackh726` who added this feature gate I think
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 4, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#132937 (a release operation synchronizes with an acquire operation)
 - rust-lang#133681 (improve TagEncoding::Niche docs, sanity check, and UB checks)
 - rust-lang#133726 (Add `core::arch::breakpoint` and test)
 - rust-lang#133768 (Remove `generic_associated_types_extended` feature gate)
 - rust-lang#133811 ([AIX] change AIX default codemodel=large)
 - rust-lang#133812 (Update wasm-component-ld to 0.5.11)
 - rust-lang#133813 (compiletest: explain that UI tests are expected not to compile by default)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 12519a6 into rust-lang:master Dec 4, 2024
6 checks passed
@rustbot rustbot added this to the 1.85.0 milestone Dec 4, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 4, 2024
Rollup merge of rust-lang#133768 - compiler-errors:gate, r=lcnr,jackh726

Remove `generic_associated_types_extended` feature gate

This PR retires nightly support for the `generic_associated_types_extended` feature. This feature hasn't received much attention in the last two years or so, and I believe the feature still remains both unsound and ICEy to use. I think that if we were to redesign and reimplement it, we'd want to first figure out how to implement it soundly, but in the mean time I'd prefer to clean this up.

r? ``@lcnr`` cc ``@jackh726`` who added this feature gate I think
fmease added a commit to fmease/rust that referenced this pull request Dec 4, 2024
…, r=oli-obk

No need to create placeholders for GAT args in confirm_object_candidate

We no longer need this logic to add placeholders for GAT args since with the removal of the `gat_extended` feature gate (rust-lang#133768) we no longer allow GATs in dyn trait anyways.

r? oli-obk
fmease added a commit to fmease/rust that referenced this pull request Dec 4, 2024
…, r=oli-obk

No need to create placeholders for GAT args in confirm_object_candidate

We no longer need this logic to add placeholders for GAT args since with the removal of the `gat_extended` feature gate (rust-lang#133768) we no longer allow GATs in dyn trait anyways.

r? oli-obk
fmease added a commit to fmease/rust that referenced this pull request Dec 5, 2024
…, r=oli-obk

No need to create placeholders for GAT args in confirm_object_candidate

We no longer need this logic to add placeholders for GAT args since with the removal of the `gat_extended` feature gate (rust-lang#133768) we no longer allow GATs in dyn trait anyways.

r? oli-obk
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 5, 2024
Rollup merge of rust-lang#133872 - compiler-errors:simplify-gat-check, r=oli-obk

No need to create placeholders for GAT args in confirm_object_candidate

We no longer need this logic to add placeholders for GAT args since with the removal of the `gat_extended` feature gate (rust-lang#133768) we no longer allow GATs in dyn trait anyways.

r? oli-obk
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Dec 6, 2024
No need to create placeholders for GAT args in confirm_object_candidate

We no longer need this logic to add placeholders for GAT args since with the removal of the `gat_extended` feature gate (rust-lang/rust#133768) we no longer allow GATs in dyn trait anyways.

r? oli-obk
@dhardy
Copy link
Contributor

dhardy commented Dec 7, 2024

Sure, I can open an issue about having GATs in dyn-compatible traits.

#81823 is related, though reads more like a (diagnostic) bug report than a feature tracker.

Personally I'd love to see support for dyn-compatible GATs with lifetime parameters.

lnicola pushed a commit to lnicola/rust-analyzer that referenced this pull request Dec 11, 2024
No need to create placeholders for GAT args in confirm_object_candidate

We no longer need this logic to add placeholders for GAT args since with the removal of the `gat_extended` feature gate (rust-lang/rust#133768) we no longer allow GATs in dyn trait anyways.

r? oli-obk
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.

6 participants