-
Notifications
You must be signed in to change notification settings - Fork 14k
Deny const auto traits #149307
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
base: main
Are you sure you want to change the base?
Deny const auto traits #149307
Conversation
|
|
8eb8894 to
809379a
Compare
| LL | const auto trait Marker {} | ||
| | ^^^^^ | ||
| | | ||
| = help: remove the `const` keyword |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could provide a structured suggestion but yeah, it's not really worth the effort.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I initially thought about that, but decided not to, mainly because not every suggestion can be fixed automatically with rustfix, for example the code sample in the original issue that triggers a panic in next-gen solvers, and because we don't really have anything meaningful to add beyond what the current message already communicates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, the next-gen solver still crashes despite this change? Right, of course.
I guess we should replace the unreachable!(…) in consider_auto_trait_candidate of HostEffectPredicate with a cx.delay_bug. Sorry, I forgot about that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, that’s a bit different from what I was aiming to accomplish in this PR, so I had planned to handle it in a follow-up PR instead (though I should have mentioned that). But since integrating the tests is straightforward, I’ve decided to include it in this PR as well: c80ab42
|
Thanks! Two small nitpicks |
44f4a9b to
0f63688
Compare
0f63688 to
3326fbd
Compare
|
@rustbot ready |
|
@bors r+ rollup |
|
Ah, we also need to replace the @bors r- |
|
@rustbot ready |
c80ab42 to
ae699c8
Compare
|
Thanks again @bors r+ |
…, r=fmease Deny const auto traits close rust-lang#149285 The AST validation now detects and rejects const auto traits. Additionally, I updated an existing test that was using `const unsafe auto trait`. r? fmease
close #149285
The AST validation now detects and rejects const auto traits. Additionally, I updated an existing test that was using
const unsafe auto trait.r? fmease