-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Get rid of HIR const checker #133321
base: master
Are you sure you want to change the base?
Get rid of HIR const checker #133321
Conversation
r? @wesleywiser rustbot has assigned @wesleywiser. Use |
Some changes occurred to the CTFE machinery cc @rust-lang/wg-const-eval |
I didn't even know we had another const checker... |
| |_____^ | ||
| | ||
= note: see issue #87575 <https://github.com/rust-lang/rust/issues/87575> for more information | ||
= help: add `#![feature(const_for)]` to the crate attributes to enable |
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.
This helpful hint seems to have been lost?
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.
Notably, enabling this gate is neither necessary nor sufficient under const traits. I should have noted that.
If and when we work out how cons stability works for const traits it may, but it's just misleading today.
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.
It's just quite frustrating to get a nightly feature error without any hint about what the feature gate is. If I first get an error saying to enable const_for, and then another error saying to enable const_trait_impl, that's a lot better IMO.
Or does the const_for gate not actually do anything right now?
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.
AFAICT it does not do anything.
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. Then it should mention const_trait_impl
, no?
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.
No. Iterator is not a const trait yet.
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.
Oh, I see.
Yeah then the new error is fine. Maybe entirely remove the const_for
symbol (and the others that were used only in the HIR const checker).
--> $DIR/try.rs:6:5 | ||
| | ||
LL | x?; | ||
| ^^ | ||
| | ||
= note: see issue #74935 <https://github.com/rust-lang/rust/issues/74935> for more information | ||
= help: add `#![feature(const_try)]` to the crate attributes to enable |
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.
And this one also got lost.
As far as I can tell, the HIR const checker was implemented in #66170 because we were not able to issue useful const error messages in the MIR const checker.
This seems to have changed in the last 5 years, probably due to work like #90532. I've tweaked the diagnostics slightly and think the error messages have gotten better in fact.
Thus I think the HIR const checker has reached the end of its usefulness, and we can retire it.
cc @RalfJung