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

Individualize feature gates for const fn invocation #43017

Merged
merged 3 commits into from Sep 16, 2017

Conversation

durka
Copy link
Contributor

@durka durka commented Jul 2, 2017

This PR changes the meaning of #![feature(const_fn)] so it is only required to declare a const fn but not to call one. Based on discussion at #24111. I was hoping we could have an FCP here in order to move that conversation forward.

This sets the stage for future stabilization of the constness of several functions in the standard library (listed below), so could someone please tag the lang team for review.

  • std::cell
    • Cell::new
    • RefCell::new
    • UnsafeCell::new
  • std::mem
    • size_of
    • align_of
  • std::ptr
    • null
    • null_mut
  • std::sync
    • atomic
      • Atomic{Bool,Ptr,Isize,Usize}::new
    • once
      • Once::new
  • primitives
    • {integer}::min_value
    • {integer}::max_value

Some other functions are const but they are also unstable or hidden, e.g. Unique::new so they don't have to be considered at this time.

After this stabilization, the following *_INIT constants in the standard library can be deprecated. I wasn't sure whether to include those deprecations in the current PR.

  • std::sync
    • atomic
      • ATOMIC_{BOOL,ISIZE,USIZE}_INIT
    • once
      • ONCE_INIT

@rust-highfive
Copy link
Collaborator

r? @eddyb

(rust_highfive has picked a reviewer for you, use r? to override)

@durka durka mentioned this pull request Jul 2, 2017
3 tasks
@eddyb
Copy link
Member

eddyb commented Jul 2, 2017

cc @rust-lang/lang

@eddyb eddyb added I-nominated T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Jul 2, 2017
@sfackler
Copy link
Member

sfackler commented Jul 2, 2017

WRT deprecations, we prefer to do those a release or two after.

@Mark-Simulacrum
Copy link
Member

[00:51:26] error: use of unstable library feature 'sort_unstable' (see issue #40585)
[00:51:26]    --> /checkout/src/test/run-pass/vector-sort-panic-safe.rs:174:30
[00:51:26]     |
[00:51:26] 174 |                 test!(input, sort_unstable_by);
[00:51:26]     |                              ^^^^^^^^^^^^^^^^
[00:51:26]     |
[00:51:26]     = help: add #![feature(sort_unstable)] to the crate attributes to enable
[00:51:26] 
[00:51:26] error: use of unstable library feature 'sort_unstable' (see issue #40585)
[00:51:26]    --> /checkout/src/test/run-pass/vector-sort-panic-safe.rs:174:30
[00:51:26]     |
[00:51:26] 174 |                 test!(input, sort_unstable_by);
[00:51:26]     |                              ^^^^^^^^^^^^^^^^
[00:51:26]     |
[00:51:26]     = help: add #![feature(sort_unstable)] to the crate attributes to enable
[00:51:26] 
[00:51:26] error: aborting due to 2 previous errors

@durka
Copy link
Contributor Author

durka commented Jul 2, 2017

Fixed botched rebase.

@durka durka force-pushed the stabilize-const-invocation branch from 358d2d2 to d0fc8fc Compare July 2, 2017 22:21
@withoutboats
Copy link
Contributor

@rfcbot fcp merge

@SimonSapin
Copy link
Contributor

After this stabilization, the following *_INIT constants in the standard library can be deprecated. I wasn't sure whether to include those deprecations in the current PR.

Do we have a mechanism yet to make something deprecated in some future version? (In this case 1.22+, I think.) It’s bothersome if Nightly warns and says to use instead something that is not available in the Stable channel yet.

@SimonSapin
Copy link
Contributor

#42859 adds size_of and align_of to the list.

@rfcbot
Copy link

rfcbot commented Jul 2, 2017

Team member @withoutboats has proposed to merge this. The next step is review by the rest of the tagged teams:

Concerns:

Once these reviewers reach consensus, this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Jul 2, 2017
@eddyb eddyb added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Jul 3, 2017
@brson brson added the relnotes Marks issues that should be documented in the release notes of the next release. label Jul 4, 2017
@brson
Copy link
Contributor

brson commented Jul 4, 2017

After this stabilization, the following *_INIT constants in the standard library can be deprecated. I wasn't sure whether to include those deprecations in the current PR.

No, please don't deprecate during the same release as the replacement. Give it a while.

@aidanhs aidanhs added the S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). label Jul 5, 2017
@joshtriplett
Copy link
Member

This is a bit of a tangent, but I'd assume this applies to core::mem::size_of and similar, not just std::mem::size_of?

@nikomatsakis
Copy link
Contributor

@rfcbot concern constness

In discussion in the @rust-lang/lang meeting, the question was raised whether we want to have the ability to control whether to stabilize the "constness" of individual functions. So, for example, one might imagine making size_of and align_of usable in constants, but not necessarily every const fn (since some of them might rely on impl details that we are not sure if we will want to stabilize).

@joshtriplett
Copy link
Member

@rust-lang/lang was generally in favor of making this change, and stabilizing size_of and align_of as const functions; the only concern was whether this might open the floodgates to stabilize numerous other library functions as const as well, and whether in adding all such functions, we might unnecessarily constrain ourselves in the development of const_fn. We'd recommend being very conservative about what in the standard library should be stabilized as const.

There was also some discussion about whether this automatically makes the constness of all const functions in the library immediately stable, which might not have been anticipated. There was discussion elsewhere about whether there might be future extensions of some such functions/traits that may be non-const.

@eddyb
Copy link
Member

eddyb commented Jul 6, 2017

@joshtriplett std::mem is a mere reexport of core::mem so yes, they are the same functions.

@durka
Copy link
Contributor Author

durka commented Jul 6, 2017 via email

@durka
Copy link
Contributor Author

durka commented Jul 6, 2017

We could also just un-constify the rest of that list, breaking nightly users until we figure out which library functions can be const. Are there any specific concerns, though?

@nikomatsakis
Copy link
Contributor

@durka

If we have to be hyper-conservative and start with just size_of/align_of, I guess that's OK, but I'm also excited to get rid of tons of lazy_statics which only exist in order to call Cell::new etc!

I have no problem being more aggressive than such a minimal starting set, but it seems like it'd be useful to have the ability to add things later that we don't stabilize immediately.

@nikomatsakis
Copy link
Contributor

That said, perhaps un-constifying is indeed a viable alternative.

@durka
Copy link
Contributor Author

durka commented Jul 10, 2017

All I can think of is adding a magic function attribute #[rustc_const_stable(since = "...")], and then basically undoing this PR :( but adding a check for the attribute here https://github.com/rust-lang/rust/pull/43017/files#diff-c2552a106550d05b69d5e07612f0f812L782

@bors
Copy link
Contributor

bors commented Sep 16, 2017

📌 Commit 332c38c has been approved by eddyb

@bors
Copy link
Contributor

bors commented Sep 16, 2017

⌛ Testing commit 332c38c with merge ae8efdc...

bors added a commit that referenced this pull request Sep 16, 2017
Individualize feature gates for const fn invocation

This PR changes the meaning of `#![feature(const_fn)]` so it is only required to declare a const fn but not to call one. Based on discussion at #24111. I was hoping we could have an FCP here in order to move that conversation forward.

This sets the stage for future stabilization of the constness of several functions in the standard library (listed below), so could someone please tag the lang team for review.

- `std::cell`
    - `Cell::new`
    - `RefCell::new`
    - `UnsafeCell::new`
- `std::mem`
    - `size_of`
    - `align_of`
- `std::ptr`
    - `null`
    - `null_mut`
- `std::sync`
    - `atomic`
        - `Atomic{Bool,Ptr,Isize,Usize}::new`
    - `once`
        - `Once::new`
- primitives
    - `{integer}::min_value`
    - `{integer}::max_value`

Some other functions are const but they are also unstable or hidden, e.g. `Unique::new` so they don't have to be considered at this time.

After this stabilization, the following `*_INIT` constants in the standard library can be deprecated. I wasn't sure whether to include those deprecations in the current PR.

- `std::sync`
    - `atomic`
        - `ATOMIC_{BOOL,ISIZE,USIZE}_INIT`
    - `once`
        - `ONCE_INIT`
@bors
Copy link
Contributor

bors commented Sep 16, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: eddyb
Pushing ae8efdc to master...

@durka
Copy link
Contributor Author

durka commented Sep 16, 2017 via email

bors-servo pushed a commit to servo/servo that referenced this pull request Sep 17, 2017
Upgrade to rustc 1.22.0-nightly (277476c4f 2017-09-16)

rust-lang/rust#43017

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18541)
<!-- Reviewable:end -->
bors-servo pushed a commit to servo/servo that referenced this pull request Sep 17, 2017
Upgrade to rustc 1.22.0-nightly (277476c4f 2017-09-16)

rust-lang/rust#43017

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18541)
<!-- Reviewable:end -->
ids1024 added a commit to ids1024/newlib that referenced this pull request Sep 18, 2017
This seems to be required now, due to rust-lang/rust#43017
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Sep 18, 2017
…-09-16) (from servo:rustup); r=SimonSapin

rust-lang/rust#43017

Source-Repo: https://github.com/servo/servo
Source-Revision: c28cf7490f2def7335c7c12e70e12e343eb1ec05

--HG--
extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear
extra : subtree_revision : 1e2b6d8c2339a47ed373f4a00dcea43348c04a99
eamsen pushed a commit to eamsen/gecko-dev that referenced this pull request Sep 18, 2017
…-09-16) (from servo:rustup); r=SimonSapin

rust-lang/rust#43017

Source-Repo: https://github.com/servo/servo
Source-Revision: c28cf7490f2def7335c7c12e70e12e343eb1ec05
aethanyc pushed a commit to aethanyc/gecko-dev that referenced this pull request Sep 19, 2017
…-09-16) (from servo:rustup); r=SimonSapin

rust-lang/rust#43017

Source-Repo: https://github.com/servo/servo
Source-Revision: c28cf7490f2def7335c7c12e70e12e343eb1ec05
JerryShih pushed a commit to JerryShih/gecko-dev that referenced this pull request Sep 19, 2017
…-09-16) (from servo:rustup); r=SimonSapin

rust-lang/rust#43017

Source-Repo: https://github.com/servo/servo
Source-Revision: c28cf7490f2def7335c7c12e70e12e343eb1ec05
@mystor
Copy link
Contributor

mystor commented Oct 29, 2017

@durka Did you ever open a bug to try to stabilize the uncontroversial const functions? It seemed from reading this that size_of and align_of, for example, are not controversial, yet I couldn't find a bug tracking their stabilization.

@durka
Copy link
Contributor Author

durka commented Oct 29, 2017

I'll try to get that process started. I guess #24111 is still tracking all the const fns -- maybe that's too confusing.

gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 1, 2019
…-09-16) (from servo:rustup); r=SimonSapin

rust-lang/rust#43017

Source-Repo: https://github.com/servo/servo
Source-Revision: c28cf7490f2def7335c7c12e70e12e343eb1ec05

UltraBlame original commit: 14d7326af64d60a6be900427b468f94c7a2d2fda
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 2, 2019
…-09-16) (from servo:rustup); r=SimonSapin

rust-lang/rust#43017

Source-Repo: https://github.com/servo/servo
Source-Revision: c28cf7490f2def7335c7c12e70e12e343eb1ec05

UltraBlame original commit: 14d7326af64d60a6be900427b468f94c7a2d2fda
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 2, 2019
…-09-16) (from servo:rustup); r=SimonSapin

rust-lang/rust#43017

Source-Repo: https://github.com/servo/servo
Source-Revision: c28cf7490f2def7335c7c12e70e12e343eb1ec05

UltraBlame original commit: 14d7326af64d60a6be900427b468f94c7a2d2fda
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet