Skip to content

Remove inline_const from INCOMPLETE_FEATURES #80349

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

Closed
wants to merge 1 commit into from
Closed

Remove inline_const from INCOMPLETE_FEATURES #80349

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Dec 24, 2020

#![feature(inline_const)] should be fully implemented now.

I didn't run tests locally because the compiler ran out of memory on my poor device, but I think it's fine because inline_const is only used in a few places.

r? @oli-obk
@rustbot label: F-inline_const

@rustbot rustbot added the F-inline_const Inline constants (aka: const blocks, const expressions, anonymous constants) label Dec 24, 2020
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 24, 2020
@oli-obk
Copy link
Contributor

oli-obk commented Dec 24, 2020

oh wow, we have no compilation failure tests for inline_const? We should definitely remedy that, but yea, not necessary in this PR.

I didn't run tests locally because the compiler ran out of memory on my poor device,

I'll wait for CI to pass, then I'll r+

@ghost
Copy link
Author

ghost commented Dec 24, 2020

oh wow, we have no compilation failure tests for inline_const? We should definitely remedy that, but yea, not necessary in this PR.

I found a feature gate test, but it doesn't contain anything related to #![allow(incomplete_features)].

@oli-obk
Copy link
Contributor

oli-obk commented Dec 24, 2020

@bors r+

Thanks for checking. I still think we should add some tests that enable the flag but cause compilation errors with const blocks, I have no clue how good/bad/weird such errors are right now.

@bors
Copy link
Collaborator

bors commented Dec 24, 2020

📌 Commit 4de0e8c has been approved by oli-obk

@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 24, 2020
@lcnr
Copy link
Contributor

lcnr commented Dec 24, 2020

we still have #78174 and #78132, both of which seem like quite fundamental challenges to me.

I think we should still keep inline_const in INCOMPLETE_FEATURES until at least #78174 is fixed.

@lcnr
Copy link
Contributor

lcnr commented Dec 24, 2020

gonna @bors r- this for now. If you disagree with my assessment feel free to approve this again.

@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 24, 2020
@ghost
Copy link
Author

ghost commented Dec 24, 2020

we still have #78174 and #78132, both of which seem like quite fundamental challenges to me.

I think we should still keep inline_const in INCOMPLETE_FEATURES until at least #78174 is fixed.

@lcnr #78174 should fail to compile, so it does not prevent correct uses of the feature. I think it should not block this PR? Also I think INCOMPLETE_FEATURES means the feature "may not be safe to use and/or cause compiler crashes", which is not the case of #78132.

Edit: Mentioned the wrong issue. Sorry.

@ghost

This comment has been minimized.

@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 Dec 24, 2020
@lcnr
Copy link
Contributor

lcnr commented Dec 24, 2020

#![feature(inline_const)]

fn main() {
    const { &1 };
}

causes an ICE and is supposed to compile. https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=50e7b8f6374b04372127fa8adc6f360c

@ghost
Copy link
Author

ghost commented Dec 24, 2020

#![feature(inline_const)]

fn main() {
    const { &1 };
}

causes an ICE and is supposed to compile. https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=50e7b8f6374b04372127fa8adc6f360c

Oh, indeed. My bad. #78174 needs updating.

@ghost ghost closed this Dec 24, 2020
@lcnr
Copy link
Contributor

lcnr commented Dec 24, 2020

To explain myself a bit better here:

I consider INCOMPLETE_FEATURES to be features which I would not recommend people to use except when experimenting. In my view inline_const is not yet working well enough for me to use it in any serious projects as it is quite easy to either break the compiler or get unexpected behavior when using it.

@ghost
Copy link
Author

ghost commented Dec 24, 2020

To explain myself a bit better here:

I consider INCOMPLETE_FEATURES to be features which I would not recommend people to use except when experimenting. In my view inline_const is not yet working well enough for me to use it in any serious projects as it is quite easy to either break the compiler or get unexpected behavior when using it.

Fair point. Maybe I'm too zealous about inline_const. 😕

@RalfJung
Copy link
Member

RalfJung commented Jan 1, 2021

I consider INCOMPLETE_FEATURES to be features which I would not recommend people to use except when experimenting. In my view inline_const is not yet working well enough for me to use it in any serious projects as it is quite easy to either break the compiler or get unexpected behavior when using it.

Now I am somewaht concerned that the standard library uses some of these features...

library/stdarch/crates/core_arch/src/lib.rs
5:#![allow(incomplete_features)]

library/alloc/src/lib.rs
73:#![allow(incomplete_features)]

library/core/src/lib.rs
65:#![allow(incomplete_features)]

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jan 3, 2021
remove allow(incomplete_features) from std

cc rust-lang#80349 (comment)

> Now I am somewhat concerned that the standard library uses some of these features...

I think it is theoretically ok to use incomplete features in the standard library or the compiler if we know that there is an already working subset and we explicitly document what we have to be careful about. Though at that point it is probably better to try and split the incomplete feature into two separate ones, similar to `min_specialization`.

Will be interesting once `feature(const_evaluatable_checked)` works well enough to imo be used in the compiler but not yet well enough to be removed from `INCOMPLETE_FEATURES`.

r? `@RalfJung`
@ghost ghost deleted the inline-const-completed branch January 15, 2021 13:39
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F-inline_const Inline constants (aka: const blocks, const expressions, anonymous constants) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants