-
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
further split up const_fn feature flag #84310
Conversation
4be8e3d
to
fbfaab2
Compare
r? @estebank (rust-highfive has picked a reviewer for you, use r? to override) |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
a38e775
to
04db4ab
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
3aee532
to
6197a4c
Compare
This comment has been minimized.
This comment has been minimized.
6197a4c
to
46d09f7
Compare
Please move the |
@oli-obk That feature gate is still used in some places, and I don't always understand what that is for -- in particular in |
replace the first one with the second one can likely just not check any feature gate, as it currently is testing for the feature gate being inactive, so there's no problem just removing the check |
That works... now I have to remove this flag from >50 tests. |
Actually I take this back, something seems wrong: after doing what you proposed, the impl<T: LambdaL> ScopedCell<T> {
#[rustc_allow_const_fn_unstable(const_fn)]
pub const fn new(value: <T as ApplyL<'static>>::Out) -> Self {
ScopedCell(Cell::new(value))
}
} |
So, I got it all to build again, but I don't think this is right. I added a new feature gate for one last use of Moreover, some errors in the |
Oh also this is now pointing to a commit of stdarch that is in my fork (to include rust-lang/stdarch#1140), I don't know if this will even build on CI... |
@@ -108,17 +108,14 @@ const fn foo37(a: bool, b: bool) -> bool { a || b } | |||
fn main() {} | |||
|
|||
impl<T: std::fmt::Debug> Foo<T> { | |||
//~^ ERROR trait bounds other than `Sized` on const fn parameters are unstable |
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 part of the diff seems wrong to me (this is also what happened in proc_macro
).
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 think the is may need its own test. The message is missing because other errors are reported first?
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.
That makes no sense to me, the order of errors shouldn't change just because I got rid of a feature flag...
This comment has been minimized.
This comment has been minimized.
It does require rustc_attrs though, so that's ok |
@Mark-Simulacrum it seems to still use the old |
📌 Commit 49054c3 has been approved by |
☀️ Test successful - checks-actions |
📣 Toolstate changed by #84310! Tested on commit b56b175. 💔 rls on windows: test-pass → build-fail (cc @Xanewok). |
Tested on commit rust-lang/rust@b56b175. Direct link to PR: <rust-lang/rust#84310> 💔 rls on windows: test-pass → build-fail (cc @Xanewok). 💔 rls on linux: test-pass → build-fail (cc @Xanewok). 💔 rustfmt on windows: test-pass → build-fail (cc @calebcartwright @topecongiro). 💔 rustfmt on linux: test-pass → build-fail (cc @calebcartwright @topecongiro).
This is necessary since rust-lang/rust#84310, otherwise new nightlies can't build cpuio. Fixes emk#8.
This continues the work on splitting up
const_fn
into separate feature flags:const_fn_trait_bound
forconst fn
with trait boundsconst_fn_unsize
for unsizing coercions inconst fn
(looks like onlydyn
unsizing is still guarded here)I don't know if there are even any things left that
const_fn
guards... at least libcore and liballoc do not need it any more.@oli-obk are you currently able to do reviews?