Stabilize transmute in constants and statics but not const fn
#72920
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rust-highfive
added
the
S-waiting-on-review
jonas-schievink
added
relnotes
oli-obk
commented
Jun 2, 2020
|
Idk who needs to sign off on this stabilization. Technically T-libs? Hypothetically just wg-const-eval? I mean this is just removing boilerplate for users for a feature already existing via |
| span, | ||
| "can only call `transmute` from const items, not `const fn`".into(), | ||
| )); | ||
| } |
There was a problem hiding this comment.
Since you only check for this in qualify_min_const_fn, transmute calls will be allowed in const fn if #![feature(const_fn)] is enabled regardless of whether the feature gate added in this PR is enabled. Is this intentional?
There was a problem hiding this comment.
I noticed that, but I don't know what our const_fn feature gate story is anymore. iirc it's "the const_fn feature will go away when we merge min_const_fn into the regular const checks"?
There was a problem hiding this comment.
I think so. I believe it's only blocked on me finding the willpower to update several hundred //~ ERROR annotations in the min_const_fn suite. Centril had some objections IIRC, but I don't think they were substantive.
In the meantime, we should check this in check_consts/validation.rs as well. Adding a structured error type for transmute and checking for it where we handle TerminatorKind::Call should be enough.
There was a problem hiding this comment.
Also we should have good test coverage for this one to ensure we don't accidentally stabilize transmute in const fn under strange circumstances...
There was a problem hiding this comment.
In the meantime, we should check this in check_consts/validation.rs as well. Adding a structured error type for transmute and checking for it where we handle TerminatorKind::Call should be enough.
@ecstatic-morse so are you saying that should happen in this PR, or should be done some time after landing this PR but before removing min_const_fn?
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
RalfJung
reviewed
Jun 2, 2020
RalfJung
reviewed
Jun 3, 2020
Since intrinsics are involved, I think T-lang needs to sign off as well. |
RalfJung
reviewed
Jun 3, 2020
RalfJung
reviewed
Jun 3, 2020
|
@rust-lang/lang This PR stabilizes I do not know of any concerns that would speak against doing this, except for impl complexity. This PR shows all the complexity needed and is fairly minimal imo. The additional code will go away once we decide to allow |
|
The idea of "transmute is permitted wherever union fields are" seems reasonable to me. I'm going to go ahead and kick off a fcp merge here -- ordinarily I'd prefer a more "formalized" stabilization report, but I think that @oli-obk's comment, plus the tests one can see in the diff here, probably suffices. @rfcbot fcp merge |
|
Team member @nikomatsakis has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), 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
added
proposed-final-comment-period
|
Discussed in triage meeting today: felt ok about this. =) |
|
Briefly, in the @rust-lang/lang meeting we were wondering why unions are disallowed in const fns, though we don't think that should block progress on this issue. |
|
There lots (and even recent) discussion on unions in const fn in #51909 TLDR is that as of right now you can't do unsafe things in const fn, and we first need to communicate the difference in safety rules in const vs non-const contexts. |
|
☔ The latest upstream changes (presumably #69478) made this pull request unmergeable. Please resolve the merge conflicts. |
I'm guessing that we'll still need to follow the FCP procedure. ping @pnkfelix, @cramertj, @scottmcm and @withoutboats there are checkboxes in #72920 (comment) for your to click or raise concerns about. |
|
What is the usual procedure for FCPs that went a few weeks without T-lang activity? |
|
@RalfJung the usual procedure sadly is to do nothing, but I'm trying to change that. I'll try to direct some attention here,. To start: |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jul 12, 2020
…arth Rollup of 10 pull requests Successful merges: - rust-lang#72920 (Stabilize `transmute` in constants and statics but not const fn) - rust-lang#73715 (debuginfo: Mangle tuples to be natvis friendly, typedef basic types) - rust-lang#74066 (Optimize is_ascii for str and [u8].) - rust-lang#74116 (Fix cross compilation of LLVM to aarch64 Windows targets) - rust-lang#74167 (linker: illumos ld does not support --eh-frame-hdr) - rust-lang#74168 (Add a help to use `in_band_lifetimes` in nightly) - rust-lang#74197 (Reword incorrect `self` token suggestion) - rust-lang#74213 (Minor refactor for rustc_resolve diagnostics match) - rust-lang#74240 (Fix rust-lang#74081 and add the test case from rust-lang#74236) - rust-lang#74241 (update miri) Failed merges: r? @ghost
flip1995
pushed a commit
to flip1995/rust
that referenced
this pull request
Jul 14, 2020
Stabilize `transmute` in constants and statics but not const fn cc rust-lang#53605 (leaving issue open so we can add `transmute` to `const fn` later) Previous attempt: rust-lang#64011 r? @RalfJung cc @rust-lang/wg-const-eval
Merged
RalfJung
pushed a commit
to Gilnaa/memoffset
that referenced
this pull request
Jul 30, 2020
Remove `const_transmute` feature flag `const_transmute` has been stabilized on current nightly, see rust-lang/rust#72920. Document `const_fn_transmute` requirement when using `offset_of!` inside a `const fn`. Add test for `offset_of!` inside a `const fn`.
netbsd-srcmastr
pushed a commit
to NetBSD/pkgsrc
that referenced
this pull request
Aug 30, 2020
according to various people on tech-pkg@, there are no problems with
the Firefox build
Version 1.46.0 (2020-08-27)
==========================
Language
--------
- [`if`, `match`, and `loop` expressions can now be used in const functions.][72437]
- [Additionally you are now also able to coerce and cast to slices (`&[T]`) in
const functions.][73862]
- [The `#[track_caller]` attribute can now be added to functions to use the
function's caller's location information for panic messages.][72445]
- [Recursively indexing into tuples no longer needs parentheses.][71322] E.g.
`x.0.0` over `(x.0).0`.
- [`mem::transmute` can now be used in static and constants.][72920] **Note**
You currently can't use `mem::transmute` in constant functions.
Compiler
--------
- [You can now use the `cdylib` target on Apple iOS and tvOS platforms.][73516]
- [Enabled static "Position Independent Executables" by default
for `x86_64-unknown-linux-musl`.][70740]
Libraries
---------
- [`mem::forget` is now a `const fn`.][73887]
- [`String` now implements `From<char>`.][73466]
- [The `leading_ones`, and `trailing_ones` methods have been stabilised for all
integer types.][73032]
- [`vec::IntoIter<T>` now implements `AsRef<[T]>`.][72583]
- [All non-zero integer types (`NonZeroU8`) now implement `TryFrom` for their
zero-able equivalent (e.g. `TryFrom<u8>`).][72717]
- [`&[T]` and `&mut [T]` now implement `PartialEq<Vec<T>>`.][71660]
- [`(String, u16)` now implements `ToSocketAddrs`.][73007]
- [`vec::Drain<'_, T>` now implements `AsRef<[T]>`.][72584]
Stabilized APIs
---------------
- [`Option::zip`]
- [`vec::Drain::as_slice`]
Cargo
-----
Added a number of new environment variables that are now available when
compiling your crate.
- [`CARGO_BIN_NAME` and `CARGO_CRATE_NAME`][cargo/8270] Providing the name of
the specific binary being compiled and the name of the crate.
- [`CARGO_PKG_LICENSE`][cargo/8325] The license from the manifest of the package.
- [`CARGO_PKG_LICENSE_FILE`][cargo/8387] The path to the license file.
Compatibility Notes
-------------------
- [The target configuration option `abi_blacklist` has been renamed
to `unsupported_abis`.][74150] The old name will still continue to work.
- [Rustc will now warn if you cast a C-like enum that implements `Drop`.][72331]
This was previously accepted but will become a hard error in a future release.
- [Rustc will fail to compile if you have a struct with
`#[repr(i128)]` or `#[repr(u128)]`.][74109] This representation is currently only
allowed on `enum`s.
- [Tokens passed to `macro_rules!` are now always captured.][73293] This helps
ensure that spans have the correct information, and may cause breakage if you
were relying on receiving spans with dummy information.
- [The InnoSetup installer for Windows is no longer available.][72569] This was
a legacy installer that was replaced by a MSI installer a few years ago but
was still being built.
- [`{f32, f64}::asinh` now returns the correct values for negative numbers.][72486]
- [Rustc will no longer accept overlapping trait implementations that only
differ in how the lifetime was bound.][72493]
- [Rustc now correctly relates the lifetime of an existential associated
type.][71896] This fixes some edge cases where `rustc` would erroneously allow
you to pass a shorter lifetime than expected.
- [Rustc now dynamically links to `libz` (also called `zlib`) on Linux.][74420]
The library will need to be installed for `rustc` to work, even though we
expect it to be already available on most systems.
- [Tests annotated with `#[should_panic]` are broken on ARMv7 while running
under QEMU.][74820]
- [Pretty printing of some tokens in procedural macros changed.][75453] The
exact output returned by rustc's pretty printing is an unstable
implementation detail: we recommend any macro relying on it to switch to a
more robust parsing system.
[75453]: rust-lang/rust#75453
[74820]: rust-lang/rust#74820
[74420]: rust-lang/rust#74420
[74109]: rust-lang/rust#74109
[74150]: rust-lang/rust#74150
[73862]: rust-lang/rust#73862
[73887]: rust-lang/rust#73887
[73466]: rust-lang/rust#73466
[73516]: rust-lang/rust#73516
[73293]: rust-lang/rust#73293
[73007]: rust-lang/rust#73007
[73032]: rust-lang/rust#73032
[72920]: rust-lang/rust#72920
[72569]: rust-lang/rust#72569
[72583]: rust-lang/rust#72583
[72584]: rust-lang/rust#72584
[72717]: rust-lang/rust#72717
[72437]: rust-lang/rust#72437
[72445]: rust-lang/rust#72445
[72486]: rust-lang/rust#72486
[72493]: rust-lang/rust#72493
[72331]: rust-lang/rust#72331
[71896]: rust-lang/rust#71896
[71660]: rust-lang/rust#71660
[71322]: rust-lang/rust#71322
[70740]: rust-lang/rust#70740
[cargo/8270]: rust-lang/cargo#8270
[cargo/8325]: rust-lang/cargo#8325
[cargo/8387]: rust-lang/cargo#8387
[`Option::zip`]: https://doc.rust-lang.org/stable/std/option/enum.Option.html#method.zip
[`vec::Drain::as_slice`]: https://doc.rust-lang.org/stable/std/vec/struct.Drain.html#method.as_slice
|
Somehow, this did not make the Rust 1.46 release notes, even though the relevant tag is set? Cc @rust-lang/release EDIT: Hm, it appears here but not in the blog post. I thought the blog post would contain all "relnotes" changes but maybe that interpretation is wrong? |
|
Yes, the blog post just highlights a select few notable changes. |
netbsd-srcmastr
pushed a commit
to NetBSD/pkgsrc
that referenced
this pull request
Oct 30, 2020
Pkgsrc changes:
* Portability patches for Illumos have been intregrated upstream,
so are no longer needed in pkgsrc.
* Adjust one other patch, and update vendor/libc cargo checksum.
Upstream changes:
Version 1.46.0 (2020-08-27)
==========================
Language
--------
- [`if`, `match`, and `loop` expressions can now be used in const functions.]
[72437]
- [Additionally you are now also able to coerce and cast to slices (`&[T]`) in
const functions.][73862]
- [The `#[track_caller]` attribute can now be added to functions to use the
function's caller's location information for panic messages.][72445]
- [Recursively indexing into tuples no longer needs parentheses.][71322] E.g.
`x.0.0` over `(x.0).0`.
- [`mem::transmute` can now be used in static and constants.][72920] **Note**
You currently can't use `mem::transmute` in constant functions.
Compiler
--------
- [You can now use the `cdylib` target on Apple iOS and tvOS platforms.][73516]
- [Enabled static "Position Independent Executables" by default
for `x86_64-unknown-linux-musl`.][70740]
Libraries
---------
- [`mem::forget` is now a `const fn`.][73887]
- [`String` now implements `From<char>`.][73466]
- [The `leading_ones`, and `trailing_ones` methods have been stabilised for all
integer types.][73032]
- [`vec::IntoIter<T>` now implements `AsRef<[T]>`.][72583]
- [All non-zero integer types (`NonZeroU8`) now implement `TryFrom` for their
zero-able equivalent (e.g. `TryFrom<u8>`).][72717]
- [`&[T]` and `&mut [T]` now implement `PartialEq<Vec<T>>`.][71660]
- [`(String, u16)` now implements `ToSocketAddrs`.][73007]
- [`vec::Drain<'_, T>` now implements `AsRef<[T]>`.][72584]
Stabilized APIs
---------------
- [`Option::zip`]
- [`vec::Drain::as_slice`]
Cargo
-----
Added a number of new environment variables that are now available when
compiling your crate.
- [`CARGO_BIN_NAME` and `CARGO_CRATE_NAME`][cargo/8270] Providing the name of
the specific binary being compiled and the name of the crate.
- [`CARGO_PKG_LICENSE`][cargo/8325] The license from the manifest of the
package.
- [`CARGO_PKG_LICENSE_FILE`][cargo/8387] The path to the license file.
Compatibility Notes
-------------------
- [The target configuration option `abi_blacklist` has been renamed
to `unsupported_abis`.][74150] The old name will still continue to work.
- [Rustc will now warn if you have a C-like enum that implements `Drop`.][72331]
This was previously accepted but will become a hard error in a future release.
- [Rustc will fail to compile if you have a struct with
`#[repr(i128)]` or `#[repr(u128)]`.][74109] This representation is currently
only allowed on `enum`s.
- [Tokens passed to `macro_rules!` are now always captured.][73293] This helps
ensure that spans have the correct information, and may cause breakage if you
were relying on receiving spans with dummy information.
- [The InnoSetup installer for Windows is no longer available.][72569] This was
a legacy installer that was replaced by a MSI installer a few years ago but
was still being built.
- [`{f32, f64}::asinh` now returns the correct values for negative numbers.]
[72486]
- [Rustc will no longer accept overlapping trait implementations that only
differ in how the lifetime was bound.][72493]
- [Rustc now correctly relates the lifetime of an existential associated
type.][71896] This fixes some edge cases where `rustc` would erroneously
allow you to pass a shorter lifetime than expected.
- [Rustc now dynamically links to `libz` (also called `zlib`) on Linux.][74420]
The library will need to be installed for `rustc` to work, even though we
expect it to be already available on most systems.
- [Tests annotated with `#[should_panic]` are broken on ARMv7 while running
under QEMU.][74820]
- [Pretty printing of some tokens in procedural macros changed.][75453] The
exact output returned by rustc's pretty printing is an unstable
implementation detail: we recommend any macro relying on it to switch to a
more robust parsing system.
[75453]: rust-lang/rust#75453
[74820]: rust-lang/rust#74820
[74420]: rust-lang/rust#74420
[74109]: rust-lang/rust#74109
[74150]: rust-lang/rust#74150
[73862]: rust-lang/rust#73862
[73887]: rust-lang/rust#73887
[73466]: rust-lang/rust#73466
[73516]: rust-lang/rust#73516
[73293]: rust-lang/rust#73293
[73007]: rust-lang/rust#73007
[73032]: rust-lang/rust#73032
[72920]: rust-lang/rust#72920
[72569]: rust-lang/rust#72569
[72583]: rust-lang/rust#72583
[72584]: rust-lang/rust#72584
[72717]: rust-lang/rust#72717
[72437]: rust-lang/rust#72437
[72445]: rust-lang/rust#72445
[72486]: rust-lang/rust#72486
[72493]: rust-lang/rust#72493
[72331]: rust-lang/rust#72331
[71896]: rust-lang/rust#71896
[71660]: rust-lang/rust#71660
[71322]: rust-lang/rust#71322
[70740]: rust-lang/rust#70740
[cargo/8270]: rust-lang/cargo#8270
[cargo/8325]: rust-lang/cargo#8325
[cargo/8387]: rust-lang/cargo#8387
[`Option::zip`]: https://doc.rust-lang.org/stable/std/option/enum.Option.html#method.zip
[`vec::Drain::as_slice`]: https://doc.rust-lang.org/stable/std/vec/struct.Drain.html#method.as_slice
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
cc #53605 (leaving issue open so we can add
transmutetoconst fnlater)Previous attempt: #64011
r? @RalfJung
cc @rust-lang/wg-const-eval