Constify `Box<T, A>` methods #91884

lilasta · 2021-12-14T02:08:27Z

Tracking issue: #92521

Most of the methods bounded on ~const. intrinsics::const_eval_select is used for handling an allocation error.

Constified API

impl<T, A: Allocator> Box<T, A> {
    pub const fn new_in(x: T, alloc: A) -> Self
    where
        A: ~const Allocator + ~const Drop;
    pub const fn try_new_in(x: T, alloc: A) -> Result<Self, AllocError>
    where
        T: ~const Drop,
        A: ~const Allocator + ~const Drop;
    pub const fn new_uninit_in(alloc: A) -> Box<mem::MaybeUninit<T>, A>
    where
        A: ~const Allocator + ~const Drop;
    pub const fn try_new_uninit_in(alloc: A) -> Result<Box<mem::MaybeUninit<T>, A>, AllocError>
    where
        A: ~const Allocator + ~const Drop;
    pub const fn new_zeroed_in(alloc: A) -> Box<mem::MaybeUninit<T>, A>
    where
        A: ~const Allocator + ~const Drop;
    pub const fn try_new_zeroed_in(alloc: A) -> Result<Box<mem::MaybeUninit<T>, A>, AllocError>
    where
        A: ~const Allocator + ~const Drop;
    pub const fn pin_in(x: T, alloc: A) -> Pin<Self>
    where
        A: 'static,
        A: 'static + ~const Allocator + ~const Drop,
    pub const fn into_boxed_slice(boxed: Self) -> Box<[T], A>;
    pub const fn into_inner(boxed: Self) -> T
    where
        Self: ~const Drop,
}

impl<T, A: Allocator> Box<MaybeUninit<T>, A> {
    pub const unsafe fn assume_init(self) -> Box<T, A>;
    pub const fn write(mut boxed: Self, value: T) -> Box<T, A>;
    pub const unsafe fn from_raw_in(raw: *mut T, alloc: A) -> Self;
    pub const fn into_raw_with_allocator(b: Self) -> (*mut T, A);
    pub const fn into_unique(b: Self) -> (Unique<T>, A);
    pub const fn allocator(b: &Self) -> &A;
    pub const fn leak<'a>(b: Self) -> &'a mut T
    where
        A: 'a;
    pub const fn into_pin(boxed: Self) -> Pin<Self>
    where
        A: 'static;
}

unsafe impl<#[may_dangle] T: ?Sized, A: Allocator> const Drop for Box<T, A>;
impl<T: ?Sized, A: Allocator> const From<Box<T, A>> for Pin<Box<T, A>>
where
    A: 'static;
impl<T: ?Sized, A: Allocator> const Deref for Box<T, A>;
impl<T: ?Sized, A: Allocator> const DerefMut for Box<T, A>;
impl<T: ?Sized, A: Allocator> const Unpin for Box<T, A> where A: 'static;

Example

pub struct ConstAllocator;

unsafe impl const Allocator for ConstAllocator {
    fn allocate(&self, layout: Layout) -> Result<NonNull<[u8]>, AllocError> {
        unsafe {
            let ptr = core::intrinsics::const_allocate(layout.size(), layout.align());
            Ok(NonNull::new_unchecked(ptr as *mut [u8; 0] as *mut [u8]))
        }
    }

    unsafe fn deallocate(&self, _ptr: NonNull<u8>, _layout: Layout) {
        /* do nothing */
    }

    fn allocate_zeroed(&self, layout: Layout) -> Result<NonNull<[u8]>, AllocError> {
        self.allocate(layout)
    }

    unsafe fn grow(
        &self,
        _ptr: NonNull<u8>,
        _old_layout: Layout,
        _new_layout: Layout,
    ) -> Result<NonNull<[u8]>, AllocError> {
        unimplemented!()
    }

    unsafe fn grow_zeroed(
        &self,
        _ptr: NonNull<u8>,
        _old_layout: Layout,
        _new_layout: Layout,
    ) -> Result<NonNull<[u8]>, AllocError> {
        unimplemented!()
    }

    unsafe fn shrink(
        &self,
        _ptr: NonNull<u8>,
        _old_layout: Layout,
        _new_layout: Layout,
    ) -> Result<NonNull<[u8]>, AllocError> {
        unimplemented!()
    }

    fn by_ref(&self) -> &Self
    where
        Self: Sized,
    {
        self
    }
}

#[test]
fn const_box() {
    const VALUE: u32 = {
        let mut boxed = Box::new_in(1u32, ConstAllocator);
        assert!(*boxed == 1);

        *boxed = 42;
        assert!(*boxed == 42);

        *boxed
    };

    assert!(VALUE == 42);
}

rust-highfive · 2021-12-14T02:08:30Z

r? @yaahc

(rust-highfive has picked a reviewer for you, use r? to override)

bors · 2021-12-15T13:20:06Z

☔ The latest upstream changes (presumably #91959) made this pull request unmergeable. Please resolve the merge conflicts.

library/alloc/src/boxed.rs

bjorn3 · 2021-12-20T17:33:16Z

library/alloc/tests/boxed.rs

+        _old_layout: Layout,
+        _new_layout: Layout,
+    ) -> Result<NonNull<[u8]>, AllocError> {
+        unimplemented!()


Please implement these.

This is a type for testing, so I omitted some implementations. Should I still implement these?

It is fine to leave them unimplemented for now, but I think they should be implemented before merging. You may be able to copy the default implementation of these methods.

I have implemented these, but the methods aren't called in the testcases, we can't check if they are implemented correctly. Also we may need to implement const_deallocate to the compiler.

Const deallocation is simply leaking the thing. The mir interpreter cleans up all dangling allocations

Also we may need to implement const_deallocate to the compiler.

I just saw you open a PR to add this, let me emphasize: This PR cannot contain any use of that intrinsic. The nature of ConstAlloc/CtfeAlloc is to leak when dropping, which means you can't realistically use it in this test allocator impl.

The nature of ConstAlloc/CtfeAlloc is to leak when dropping

And why is that?
I don't think this should be the case, at least for allocations that are created and then deallocated within the same const-evalaution. (Of course, at runtime deallocation is a NOP, since static memory cannot be deallocated and it is shared among all the copies of the constant anyway.)

Well, looks like const_eval_select can resolve this problem faily easily.

I don't think that is enough -- if one const works on something that was allocated in a different const, deallocation should still be a NOP.

library/alloc/src/alloc.rs

fee1-dead · 2021-12-23T12:24:06Z

cc @rust-lang/wg-const-eval

Should we do this before offering ConstAlloc as an implementation and figure out what the best approach to const alloc is?

oli-obk · 2021-12-23T13:54:03Z

~~Yea, I would prefer a ConstAlloc to play with first, too. But I'll have a look at the impl, maybe we could merge it and have an arena allocator test to show it works without allowing GlobalAlloc ?~~

oli-obk · 2021-12-23T13:54:16Z

r? @oli-obk

oli-obk · 2021-12-23T13:57:40Z

Oh hold up. I should've read the PR first. I think this is fine as it is? Or am I missing sth? Even if we go a magic route, we'll want this, right?

library/alloc/src/boxed.rs

RalfJung · 2021-12-23T14:39:48Z

library/alloc/tests/boxed.rs

+        match layout.size() {
+            0 => { /* do nothing */ }
+            _ => { /* do nothing too */ }
+        }


I think if we have something like const_allocate dynamically creating allocations during const-eval, we should also have a corresponding const_deallocate to get rid of them again. This will improve UB detection and reduce memory consumption during CTFE.

oli-obk · 2021-12-23T16:30:54Z

@woppopo how motivated are you feeling right now 😄? If a lot, could you create a ui test showing how to use a Box with an array-backed arena allocator?

library/alloc/src/boxed.rs

lilasta · 2021-12-25T09:25:31Z

@woppopo how motivated are you feeling right now 😄? If a lot, could you create a ui test showing how to use a Box with an array-backed arena allocator?

@oli-obk
I encountered an ICE... 😢
~~#92272 (duplicate)~~
#78459

oli-obk · 2021-12-25T10:33:44Z

Oh no 😄 I guess we'll have to wait for that bug to be fixed or for the same constify work to be done for vec

So... a tracking issue with a reference to this problem is all that's needed for this PR

lilasta · 2022-01-03T07:25:30Z

Oh no 😄 I guess we'll have to wait for that bug to be fixed or for the same constify work to be done for vec
So... a tracking issue with a reference to this problem is all that's needed for this PR.

ConstAllocator used for the test is a zero sized type, this PR probably isn't affected by the problem.

oli-obk · 2022-01-03T11:22:25Z

Oh yea, definitely

The PR just needs a tracking issue and the feature gates should reference that

oli-obk · 2022-01-03T15:31:54Z

@bors r+ rollup

lilasta · 2022-01-03T15:35:46Z

I've added tracking issues. By the way, I just realized that we can test Box::new_in with array-backed allocator by using static variables internally...
https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=f1ed3eb26a714e269439c7136b6043dd

lilasta · 2022-01-03T15:41:02Z

I made a mistake in previous commit, re-pushed. Sorry, please re-approve...

RalfJung · 2022-01-03T16:00:37Z

@bors r=oli-obk

bors · 2022-01-03T16:00:38Z

📌 Commit c9d2d3c has been approved by oli-obk

RalfJung · 2022-01-03T16:39:14Z

@bors r- delegate+
please do bors r=oli-obk when CI is green :)

bors · 2022-01-03T16:39:17Z

✌️ @woppopo can now approve this pull request

lilasta · 2022-01-03T16:41:03Z

I'm so sorry to bother you again🙇...

lilasta · 2022-01-03T17:21:37Z

@bors r=oli-obk

bors · 2022-01-03T17:21:38Z

📌 Commit 51e4291 has been approved by oli-obk

Constify `Box<T, A>` methods Tracking issue: none yet Most of the methods bounded on `~const`. `intrinsics::const_eval_select` is used for handling an allocation error. <details><summary>Constified API</summary> ```rust impl<T, A: Allocator> Box<T, A> { pub const fn new_in(x: T, alloc: A) -> Self where A: ~const Allocator + ~const Drop; pub const fn try_new_in(x: T, alloc: A) -> Result<Self, AllocError> where T: ~const Drop, A: ~const Allocator + ~const Drop; pub const fn new_uninit_in(alloc: A) -> Box<mem::MaybeUninit<T>, A> where A: ~const Allocator + ~const Drop; pub const fn try_new_uninit_in(alloc: A) -> Result<Box<mem::MaybeUninit<T>, A>, AllocError> where A: ~const Allocator + ~const Drop; pub const fn new_zeroed_in(alloc: A) -> Box<mem::MaybeUninit<T>, A> where A: ~const Allocator + ~const Drop; pub const fn try_new_zeroed_in(alloc: A) -> Result<Box<mem::MaybeUninit<T>, A>, AllocError> where A: ~const Allocator + ~const Drop; pub const fn pin_in(x: T, alloc: A) -> Pin<Self> where A: 'static, A: 'static + ~const Allocator + ~const Drop, pub const fn into_boxed_slice(boxed: Self) -> Box<[T], A>; pub const fn into_inner(boxed: Self) -> T where Self: ~const Drop, } impl<T, A: Allocator> Box<MaybeUninit<T>, A> { pub const unsafe fn assume_init(self) -> Box<T, A>; pub const fn write(mut boxed: Self, value: T) -> Box<T, A>; pub const unsafe fn from_raw_in(raw: *mut T, alloc: A) -> Self; pub const fn into_raw_with_allocator(b: Self) -> (*mut T, A); pub const fn into_unique(b: Self) -> (Unique<T>, A); pub const fn allocator(b: &Self) -> &A; pub const fn leak<'a>(b: Self) -> &'a mut T where A: 'a; pub const fn into_pin(boxed: Self) -> Pin<Self> where A: 'static; } unsafe impl<#[may_dangle] T: ?Sized, A: Allocator> const Drop for Box<T, A>; impl<T: ?Sized, A: Allocator> const From<Box<T, A>> for Pin<Box<T, A>> where A: 'static; impl<T: ?Sized, A: Allocator> const Deref for Box<T, A>; impl<T: ?Sized, A: Allocator> const DerefMut for Box<T, A>; impl<T: ?Sized, A: Allocator> const Unpin for Box<T, A> where A: 'static; ``` </details> <details><summary>Example</summary> ```rust pub struct ConstAllocator; unsafe impl const Allocator for ConstAllocator { fn allocate(&self, layout: Layout) -> Result<NonNull<[u8]>, AllocError> { unsafe { let ptr = core::intrinsics::const_allocate(layout.size(), layout.align()); Ok(NonNull::new_unchecked(ptr as *mut [u8; 0] as *mut [u8])) } } unsafe fn deallocate(&self, _ptr: NonNull<u8>, _layout: Layout) { /* do nothing */ } fn allocate_zeroed(&self, layout: Layout) -> Result<NonNull<[u8]>, AllocError> { self.allocate(layout) } unsafe fn grow( &self, _ptr: NonNull<u8>, _old_layout: Layout, _new_layout: Layout, ) -> Result<NonNull<[u8]>, AllocError> { unimplemented!() } unsafe fn grow_zeroed( &self, _ptr: NonNull<u8>, _old_layout: Layout, _new_layout: Layout, ) -> Result<NonNull<[u8]>, AllocError> { unimplemented!() } unsafe fn shrink( &self, _ptr: NonNull<u8>, _old_layout: Layout, _new_layout: Layout, ) -> Result<NonNull<[u8]>, AllocError> { unimplemented!() } fn by_ref(&self) -> &Self where Self: Sized, { self } } #[test] fn const_box() { const VALUE: u32 = { let mut boxed = Box::new_in(1u32, ConstAllocator); assert!(*boxed == 1); *boxed = 42; assert!(*boxed == 42); *boxed }; assert!(VALUE == 42); } ``` </details>

…askrgr Rollup of 7 pull requests Successful merges: - rust-lang#91754 (Modifications to `std::io::Stdin` on Windows so that there is no longer a 4-byte buffer minimum in read().) - rust-lang#91884 (Constify `Box<T, A>` methods) - rust-lang#92107 (Actually set IMAGE_SCN_LNK_REMOVE for .rmeta) - rust-lang#92456 (Make the documentation of builtin macro attributes accessible) - rust-lang#92507 (Suggest single quotes when char expected, str provided) - rust-lang#92525 (intra-doc: Make `Receiver::into_iter` into a clickable link) - rust-lang#92532 (revert rust-lang#92254 "Bump gsgdt to 0.1.3") Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

Add `intrinsics::const_deallocate` Tracking issue: rust-lang#79597 Related: rust-lang#91884 This allows deallocation of a memory allocated by `intrinsics::const_allocate`. At the moment, this can be only used to reduce memory usage, but in the future this may be useful to detect memory leaks (If an allocated memory remains after evaluation, raise an error...?).

steffahn · 2022-03-10T14:08:55Z

FYI, y'all missed #94804 during review.

steffahn · 2022-03-10T17:50:36Z

library/alloc/src/boxed.rs

@@ -1908,7 +1951,8 @@ impl<T: ?Sized, A: Allocator> AsMut<T> for Box<T, A> {
 *  could have a method to project a Pin<T> from it.
 */
 #[stable(feature = "pin", since = "1.33.0")]
-impl<T: ?Sized, A: Allocator> Unpin for Box<T, A> where A: 'static {}
+#[rustc_const_unstable(feature = "const_box", issue = "92521")]
+impl<T: ?Sized, A: Allocator> const Unpin for Box<T, A> where A: 'static {}


What is const Unpin supposed to mean?

It doesn't make sense to implement a marker trait as const, but I made it const with the intention of indicating that Box can be unpinned even at compile-time.

rust-highfive assigned yaahc Dec 14, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 14, 2021

This comment has been minimized.

Sign in to view

bjorn3 reviewed Dec 20, 2021

View reviewed changes

library/alloc/src/boxed.rs Outdated Show resolved Hide resolved

bjorn3 reviewed Dec 20, 2021

View reviewed changes

This comment has been minimized.

Sign in to view

lilasta commented Dec 23, 2021

View reviewed changes

library/alloc/src/alloc.rs Outdated Show resolved Hide resolved

lilasta marked this pull request as ready for review December 23, 2021 04:10

Constify Box<T, A> methods

eb4fc64

rust-highfive assigned oli-obk and unassigned yaahc Dec 23, 2021

fee1-dead reviewed Dec 23, 2021

View reviewed changes

library/alloc/src/boxed.rs Show resolved Hide resolved

RalfJung reviewed Dec 23, 2021

View reviewed changes

oli-obk requested changes Dec 23, 2021

View reviewed changes

library/alloc/src/boxed.rs Outdated Show resolved Hide resolved

library/alloc/src/boxed.rs Show resolved Hide resolved

lilasta mentioned this pull request Dec 25, 2021

Add intrinsics::const_deallocate #92274

Merged

This was referenced Jan 3, 2022

Tracking Issue for const_box #92521

Open

Tracking Issue for const_alloc_error #92523

Open

bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jan 3, 2022

Add tracking issues (const_box, const_alloc_error)

c9d2d3c

This comment has been minimized.

Sign in to view

Fix a compile error when no_global_oom_handling

51e4291

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 Jan 3, 2022

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 3, 2022

matthiaskrgr mentioned this pull request Jan 3, 2022

Rollup of 6 pull requests #92542

Closed

matthiaskrgr mentioned this pull request Jan 4, 2022

Rollup of 7 pull requests #92556

Merged

bors merged commit c7125ba into rust-lang:master Jan 4, 2022

rustbot added this to the 1.59.0 milestone Jan 4, 2022

lilasta deleted the const_box branch January 5, 2022 01:30

steffahn reviewed Mar 10, 2022

View reviewed changes

Constify Box<T, A> methods #91884

Constify Box<T, A> methods #91884

Conversation

lilasta commented Dec 14, 2021 • edited Loading

rust-highfive commented Dec 14, 2021

This comment has been minimized.

bors commented Dec 15, 2021

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

fee1-dead commented Dec 23, 2021 • edited Loading

oli-obk commented Dec 23, 2021 • edited Loading

oli-obk commented Dec 23, 2021

oli-obk commented Dec 23, 2021

Choose a reason for hiding this comment

oli-obk commented Dec 23, 2021 • edited Loading

lilasta commented Dec 25, 2021 • edited Loading

oli-obk commented Dec 25, 2021

lilasta commented Jan 3, 2022

oli-obk commented Jan 3, 2022

oli-obk commented Jan 3, 2022

lilasta commented Jan 3, 2022

lilasta commented Jan 3, 2022

RalfJung commented Jan 3, 2022

bors commented Jan 3, 2022

This comment has been minimized.

RalfJung commented Jan 3, 2022

bors commented Jan 3, 2022

lilasta commented Jan 3, 2022

lilasta commented Jan 3, 2022

bors commented Jan 3, 2022

steffahn commented Mar 10, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Constify `Box<T, A>` methods #91884

Constify `Box<T, A>` methods #91884

lilasta commented Dec 14, 2021 •

edited

Loading

fee1-dead commented Dec 23, 2021 •

edited

Loading

oli-obk commented Dec 23, 2021 •

edited

Loading

oli-obk commented Dec 23, 2021 •

edited

Loading

lilasta commented Dec 25, 2021 •

edited

Loading

steffahn commented Mar 10, 2022 •

edited

Loading