Skip to content

Support vec![const { ... }; n] syntax #133412

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions library/alloc/src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ macro_rules! vec {
() => (
$crate::vec::Vec::new()
);
(const $elem:block; $n:expr) => (
// SAFETY: The `const` keyword asserts the value being the result of a const expression.
unsafe { $crate::vec::from_const_elem(const $elem, $n) }
);
($elem:expr; $n:expr) => (
$crate::vec::from_elem($elem, $n)
);
Expand Down
51 changes: 51 additions & 0 deletions library/alloc/src/vec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3183,6 +3183,57 @@ pub fn from_elem_in<T: Clone, A: Allocator>(elem: T, n: usize, alloc: A) -> Vec<
<T as SpecFromElem>::from_elem(elem, n, alloc)
}

/// # Safety
///
/// `elem` must be the result of some const expression.
#[doc(hidden)]
#[cfg(not(no_global_oom_handling))]
#[unstable(feature = "vec_of_const_expr", issue = "none")]
#[track_caller]
pub unsafe fn from_const_elem<T>(elem: T, n: usize) -> Vec<T> {
// SAFETY: Caller has guaranteed `elem` being the result of some const expression.
unsafe { from_const_elem_in(elem, n, Global) }
}

/// # Safety
///
/// `elem` must be the result of some const expression.
#[doc(hidden)]
#[cfg(not(no_global_oom_handling))]
#[unstable(feature = "vec_of_const_expr", issue = "none")]
#[track_caller]
unsafe fn from_const_elem_in<T, A: Allocator>(elem: T, n: usize, alloc: A) -> Vec<T, A> {
/// # Safety
///
/// `value` must points to a valid `T` value that is the result of some const expression.
unsafe fn fill_const_value<T>(buffer: &mut [MaybeUninit<T>], value: *const T) {
for target in buffer {
// SAFETY: If `value` is the result of some const expression, we can make as many
// copies as needed.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an interesting argument that I have not seen before. I can't see anything immediately wrong with it, but paging in @rust-lang/opsem for extra scrutiny.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be the case with the current const capabilities, but I'm not 100% sure it holds with every possible extension, namely leaking const allocation to runtime. I think the rules around const object unification would make this a valid execution, but I don't have all the relevant info in cache.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do intent that consts can still be implemented as many copies of the same thing for codegen. So whatever we do with const allocation should support that.

But it may be good to put a comment in the source here explaining that this is the standard library doing privileged reasoning, and not a stable guarantee.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have updated the safety comment. Please let me know if there is anything missing or inaccurate.

unsafe { target.write(ptr::read(value)) };
}
}

// Avoid calling the destructor of `elem`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This raises the interesting question whether the destructor should be executed at all for vec![const { ... }; 0]. For 0-length arrays, our current behavior here is rather strange: #79580.

Copy link
Contributor Author

@EFanZh EFanZh Nov 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested the array of const expression case, the destructor is not called: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=f9ffe1eb972bc415b0e6bf6bf48c18c1. This is consistent with the current implementation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. Would be good to have a test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test added.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good!

let elem = ManuallyDrop::new(elem);
let elem_ptr = ptr::from_ref(&*elem);
let mut result = Vec::<T, A>::with_capacity_in(n, alloc);
let buffer_ptr = result.as_mut_ptr().cast::<MaybeUninit<T>>();

// SAFETY: `with_capacity_in` makes sure the capacity is at least `n`, so we can make a buffer
// of length `n` out of it.
let buffer = unsafe { slice::from_raw_parts_mut(buffer_ptr, n) };

// SAFETY: Caller has guaranteed `elem` being the result of some const expression.
unsafe { fill_const_value(buffer, elem_ptr) };

// SAFETY: We have initialized exactly `n` values at the start of the buffer, so we are safe to
// set the length accordingly.
unsafe { result.set_len(n) };

result
}

#[cfg(not(no_global_oom_handling))]
trait ExtendFromWithinSpec {
/// # Safety
Expand Down
1 change: 1 addition & 0 deletions library/alloc/tests/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#![cfg_attr(not(bootstrap), feature(strict_provenance_lints))]
#![feature(drain_keep_rest)]
#![feature(local_waker)]
#![feature(vec_of_const_expr)]
#![feature(vec_pop_if)]
#![feature(unique_rc_arc)]
#![feature(macro_metavar_expr_concat)]
Expand Down
57 changes: 57 additions & 0 deletions library/alloc/tests/vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2311,6 +2311,63 @@ fn test_vec_macro_repeat() {
assert_eq!(vec![el; n], vec![Box::new(1), Box::new(1), Box::new(1)]);
}

#[test]
fn test_vec_macro_repeat_const() {
#[derive(Eq, PartialEq, Debug)]
struct Item {
x: u64,
y: u32, // Paddings are added to test the uninitialized bytes case.
}

impl Clone for Item {
fn clone(&self) -> Self {
panic!("no clone should be called");
}
}

const ITEM: Item = Item { x: 2, y: 3 };

assert_eq!(vec![const { ITEM }; 0], vec![ITEM; 0]);
assert_eq!(vec![const { ITEM }; 1], vec![ITEM]);
assert_eq!(vec![const { ITEM }; 2], vec![ITEM, ITEM]);
assert_eq!(vec![const { ITEM }; 3], vec![ITEM, ITEM, ITEM]);
}

#[test]
fn test_vec_macro_repeat_const_drop_behavior() {
thread_local! { static DROP_COUNT: Cell<usize> = const { Cell::new(0) } };

fn with_drop_count_scope(count: usize, f: impl FnOnce()) {
struct RestoreOldDropCount(usize);

impl Drop for RestoreOldDropCount {
fn drop(&mut self) {
DROP_COUNT.set(self.0);
}
}

let _restore_old_drop_count = RestoreOldDropCount(DROP_COUNT.replace(count));

f();
}

struct DropCounter;

impl Drop for DropCounter {
fn drop(&mut self) {
DROP_COUNT.set(DROP_COUNT.get().checked_add(1).unwrap());
}
}

for n in 0..10 {
with_drop_count_scope(0, || {
_ = vec![const { DropCounter }; n];

assert_eq!(DROP_COUNT.get(), n);
});
}
}

#[test]
fn test_vec_swap() {
let mut a: Vec<isize> = vec![0, 1, 2, 3, 4, 5, 6];
Expand Down
Loading