Make Layout::new const #66254
Make Layout::new const #66254
Conversation
|
r? @KodrAus (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
jonas-schievink
added
relnotes
Centril
added
the
needs-fcp
|
cc @rust-lang/libs for [needs-fcp] |
|
@rfcbot fcp merge |
|
Team member @sfackler 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
|
🔔 This is now entering its final comment period, as per the review above. 🔔 |
rfcbot
removed
the
proposed-final-comment-period
rfcbot
added
finished-final-comment-period
|
The final comment period, with a disposition to merge, as per the review above, is now complete. As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed. The RFC will be merged soon. |
|
@bors r+ |
|
📌 Commit e8c53a6 has been approved by |
|
🌲 The tree is currently closed for pull requests below priority 1000, this pull request will be tested once the tree is reopened |
bors
added
S-waiting-on-bors
|
⌛ Testing commit e8c53a6 with merge ca7e02a1ee5dc9691042789d473f1b574af59be8... |
|
Your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. 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 |
|
💔 Test failed - checks-azure |
bors
added
S-waiting-on-review
|
☔ The latest upstream changes (presumably #67540) made this pull request unmergeable. Please resolve the merge conflicts. |
bors
added
S-waiting-on-author
|
Unbeknown I did the same thing (and more) in #67494 ; should I wait for this to be rebased and resubmit? |
|
well this stabilizes it, while your PR just makes it const on nightly. I guess it would be best to wait for this PR and then stabilize the rest. If you want you can already change your PR to just not touch |
1 similar comment
|
well this stabilizes it, while your PR just makes it const on nightly. I guess it would be best to wait for this PR and then stabilize the rest. If you want you can already change your PR to just not touch |
Constify Result r? @oli-obk This is just the `Result`-part of rust-lang#67494 which I'll resubmit once rust-lang#66254 has landed.
Constify Result r? @oli-obk This is just the `Result`-part of rust-lang#67494 which I'll resubmit once rust-lang#66254 has landed.
|
Rebased again. |
|
@bors r=KodrAus |
|
📌 Commit d860def has been approved by |
bors
added
S-waiting-on-bors
Make Layout::new const
This seems like a reasonable change to make. If we don't provide `Layout::new::<T>` as `const`, then users can just instead do the more error prone `Layout::from_size_align_unchecked(mem::size_of::<T>(), mem::align_of::<T>())` for the same effect and an extra `unsafe { }` incantation.
Rollup of 10 pull requests Successful merges: - #66254 (Make Layout::new const) - #67122 (Do not deduplicate diagnostics in UI tests) - #67358 (Add HashSet::get_or_insert_owned) - #67725 (Simplify into_key_slice_mut) - #67935 (Relax the Sized bounds on Pin::map_unchecked(_mut)) - #67967 (Delay bug to prevent ICE in MIR borrowck) - #67975 (Export public scalar statics in wasm) - #68006 (Recognise riscv64 in compiletest) - #68040 (Cleanup) - #68054 (doc: add Null-unchecked version section to mut pointer as_mut method) Failed merges: - #67258 (Introduce `X..`, `..X`, and `..=X` range patterns) r? @ghost
Tracking issue rust-lang#67521, Layout::new in rust-lang#66254
| let (size, align) = size_align::<T>(); | ||
| // Note that the align is guaranteed by rustc to be a power of two and | ||
| // the size+align combo is guaranteed to fit in our address space. As a | ||
| // result use the unchecked constructor here to avoid inserting code | ||
| // that panics if it isn't optimized well enough. | ||
| debug_assert!(Layout::from_size_align(size, align).is_ok()); |
There was a problem hiding this comment.
Now that we have conditionals in const, can we re-add the debug_assert!?
There was a problem hiding this comment.
Given that the stdlib is always compiled in release mode, it's not too useful to re-add.
But yes, given feature(const_fn, const_if_match, and const_panic), this debug assert can be made to compile. (Ideally it'd also have const fn Result::is_ok to avoid using matches! directly.)
There was a problem hiding this comment.
We have some CI runners that have debug assertions enabled; it is not uncommon for rustc devs to enable debug assertions locally; Miri will hopefully enable libstd debug assertions eventually. So, there is some use to these (and the libstd is using them in plenty of places).
There was a problem hiding this comment.
is_ok was recently made const in #67685
There was a problem hiding this comment.
@RalfJung For some reason, while I can get debug_assert! in const_fn on the playground, any attempt to add it in libcore, even #[cfg(not(bootstrap))] is giving me error[E0723]: loops and conditional expressions are not stable in const fn.
diff --git a/src/libcore/alloc.rs b/src/libcore/alloc.rs
index a04e75bc7ce..a679fe160a8 100644
--- a/src/libcore/alloc.rs
+++ b/src/libcore/alloc.rs
@@ -131,6 +131,8 @@ impl Layout {
// the size+align combo is guaranteed to fit in our address space. As a
// result use the unchecked constructor here to avoid inserting code
// that panics if it isn't optimized well enough.
+ #[cfg(not(bootstrap))]
+ debug_assert!(Layout::from_size_align(size, align).is_ok());
unsafe { Layout::from_size_align_unchecked(size, align) }
}
diff --git a/src/libcore/lib.rs b/src/libcore/lib.rs
index bca96b77812..00ddbb923e2 100644
--- a/src/libcore/lib.rs
+++ b/src/libcore/lib.rs
@@ -82,6 +82,7 @@
#![feature(const_int_pow)]
#![feature(constctlz)]
#![feature(const_panic)]
+#![feature(const_fn)]
#![feature(const_fn_union)]
#![feature(const_generics)]
#![feature(const_ptr_offset_from)]error[E0723]: loops and conditional expressions are not stable in const fn
--> src\libcore\macros\mod.rs:184:23
|
183 | / macro_rules! debug_assert {
184 | | ($($arg:tt)*) => (if $crate::cfg!(debug_assertions) { $crate::assert!($($arg)*); })
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
185 | | }
| |_- in this expansion of `debug_assert!`
|
::: src\libcore\alloc.rs:135:9
|
135 | debug_assert!(Layout::from_size_align(size, align).is_ok());
| ------------------------------------------------------------ in this macro invocation
|
= note: see issue #57563 <https://github.com/rust-lang/rust/issues/57563> for more information
= help: add `#![feature(const_fn)]` to the crate attributes to enable
There was a problem hiding this comment.
@CAD97 Stable const fn cannot use unstable features. There's allow_internal_unstable to work around this, but at this point we might not yet want to stabilize methods that use these features internally. OTOH this is just a debug assertion so it seems fine to me.
Also @oli-obk is the better person for such questions. :) Or just ping the entire team: @rust-lang/wg-const-eval
There was a problem hiding this comment.
Actually looks like we already have stable const fn that opt into using conditionals, so just adding #[allow_internal_unstable(const_if_match)] should be fine.
There was a problem hiding this comment.
It's not fine. The stable const fns that use conditionals are just refactorings of hacks that were written without conditionals before, but they do not use more expressive power than what is available on stable, which, as far as I can tell, this would.
There was a problem hiding this comment.
This would just use it for a debug assertion though, which we can remove again any time if we have to.
Version 1.42.0 (2020-03-12)
==========================
Language
--------
- [You can now use the slice pattern syntax with subslices.][67712] e.g.
```rust
fn foo(words: &[&str]) {
match words {
["Hello", "World", "!", ..] => println!("Hello World!"),
["Foo", "Bar", ..] => println!("Baz"),
rest => println!("{:?}", rest),
}
}
```
- [You can now use `#[repr(transparent)]` on univariant `enum`s.][68122] Meaning
that you can create an enum that has the exact layout and ABI of the type
it contains.
- [There are some *syntax-only* changes:][67131]
- `default` is syntactically allowed before items in `trait` definitions.
- Items in `impl`s (i.e. `const`s, `type`s, and `fn`s) may syntactically
leave out their bodies in favor of `;`.
- Bounds on associated types in `impl`s are now syntactically allowed
(e.g. `type Foo: Ord;`).
- `...` (the C-variadic type) may occur syntactically directly as the type of
any function parameter.
These are still rejected *semantically*, so you will likely receive an error
but these changes can be seen and parsed by procedural macros and
conditional compilation.
Compiler
--------
- [Added tier 2* support for `armv7a-none-eabi`.][68253]
- [Added tier 2 support for `riscv64gc-unknown-linux-gnu`.][68339]
- [`Option::{expect,unwrap}` and
`Result::{expect, expect_err, unwrap, unwrap_err}` now produce panic messages
pointing to the location where they were called, rather than
`core`'s internals. ][67887]
* Refer to Rust's [platform support page][forge-platform-support] for more
information on Rust's tiered platform support.
Libraries
---------
- [`iter::Empty<T>` now implements `Send` and `Sync` for any `T`.][68348]
- [`Pin::{map_unchecked, map_unchecked_mut}` no longer require the return type
to implement `Sized`.][67935]
- [`io::Cursor` now derives `PartialEq` and `Eq`.][67233]
- [`Layout::new` is now `const`.][66254]
- [Added Standard Library support for `riscv64gc-unknown-linux-gnu`.][66899]
Stabilized APIs
---------------
- [`CondVar::wait_while`]
- [`CondVar::wait_timeout_while`]
- [`DebugMap::key`]
- [`DebugMap::value`]
- [`ManuallyDrop::take`]
- [`matches!`]
- [`ptr::slice_from_raw_parts_mut`]
- [`ptr::slice_from_raw_parts`]
Cargo
-----
- [You no longer need to include `extern crate proc_macro;` to be able to
`use proc_macro;` in the `2018` edition.][cargo/7700]
Compatibility Notes
-------------------
- [`Error::description` has been deprecated, and its use will now produce a
warning.][66919] It's recommended to use `Display`/`to_string` instead.
- [`use $crate;` inside macros is now a hard error.][37390] The compiler
emitted forward compatibility warnings since Rust 1.14.0.
- [As previously announced, this release reduces the level of support for
32-bit Apple targets to tier 3.][apple-32bit-drop]. This means that the
source code is still available to build, but the targets are no longer tested
and no release binary is distributed by the Rust project. Please refer to the
linked blog post for more information.
[37390]: rust-lang/rust#37390
[68253]: rust-lang/rust#68253
[68348]: rust-lang/rust#68348
[67935]: rust-lang/rust#67935
[68339]: rust-lang/rust#68339
[68122]: rust-lang/rust#68122
[67712]: rust-lang/rust#67712
[67887]: rust-lang/rust#67887
[67131]: rust-lang/rust#67131
[67233]: rust-lang/rust#67233
[66899]: rust-lang/rust#66899
[66919]: rust-lang/rust#66919
[66254]: rust-lang/rust#66254
[cargo/7700]: rust-lang/cargo#7700
[`DebugMap::key`]: https://doc.rust-lang.org/stable/std/fmt/struct.DebugMap.html#method.key
[`DebugMap::value`]: https://doc.rust-lang.org/stable/std/fmt/struct.DebugMap.html#method.value
[`ManuallyDrop::take`]: https://doc.rust-lang.org/stable/std/mem/struct.ManuallyDrop.html#method.take
[`matches!`]: https://doc.rust-lang.org/stable/std/macro.matches.html
[`ptr::slice_from_raw_parts_mut`]: https://doc.rust-lang.org/stable/std/ptr/fn.slice_from_raw_parts_mut.html
[`ptr::slice_from_raw_parts`]: https://doc.rust-lang.org/stable/std/ptr/fn.slice_from_raw_parts.html
[`CondVar::wait_while`]: https://doc.rust-lang.org/stable/std/sync/struct.Condvar.html#method.wait_while
[`CondVar::wait_timeout_while`]: https://doc.rust-lang.org/stable/std/sync/struct.Condvar.html#method.wait_timeout_while
This seems like a reasonable change to make. If we don't provide
Layout::new::<T>asconst, then users can just instead do the more error proneLayout::from_size_align_unchecked(mem::size_of::<T>(), mem::align_of::<T>())for the same effect and an extraunsafe { }incantation.