rust icon indicating copy to clipboard operation
rust copied to clipboard

Add `fn into_raw_with_allocator` to Rc/Arc/Weak.

Open zachs18 opened this issue 1 month ago • 5 comments

Split out from #119761

Add fn into_raw_with_allocator for Rc/rc::Weak[^1]/Arc/sync::Weak.

  • Pairs with from_raw_in (which already exists on all 4 types).
  • Name matches Box::into_raw_with_allocator.
  • Associated fns on Rc/Arc, methods on Weaks.
Future PR/ACP

As a follow-on to this PR, I plan to make a PR/ACP later to move into_raw(_parts) from Container<_, A: Allocator> to only Container<_, Global> (where Container = Vec/Box/Rc/rc::Weak/Arc/sync::Weak) so that users of non-Global allocators have to explicitly handle the allocator when using into_raw-like APIs.

The current behaviors of stdlib containers are inconsistent with respect to what happens to the allocator when into_raw is called (which does not return the allocator)

Type into_raw currently callable with behavior of into_raw
Box any allocator allocator is dropped
Vec any allocator allocator is forgotten
Arc/Rc/Weak any allocator allocator is forgotten(Arc) (sync::Weak) (Rc) (rc::Weak)

In my opinion, neither implicitly dropping nor implicitly forgetting the allocator is ideal; dropping it could immediately invalidate the returned pointer, and forgetting it could unintentionally leak memory. My (to-be) proposed solution is to just forbid calling into_raw(_parts) on containers with non-Global allocators, and require calling into_raw_with_allocator(/Vec::into_raw_parts_with_alloc)

[^1]: Technically, rc::Weak::into_raw_with_allocator is not newly added, as it was modified and renamed from rc::Weak::into_raw_and_alloc.

zachs18 avatar May 13 '24 22:05 zachs18

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum. They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot avatar May 13 '24 22:05 rustbot

@rustbot label +A-allocators

zachs18 avatar May 13 '24 22:05 zachs18

The job x86_64-gnu-tools failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
   Compiling cfg-if v1.0.0
   Compiling adler v1.0.2
   Compiling rustc-demangle v0.1.24
   Compiling unwind v0.0.0 (/checkout/library/unwind)
error[E0599]: the function or associated item `allocator` exists for struct `Rc<T, A>`, but its trait bounds were not satisfied
     |
317  | / pub struct Rc<
317  | / pub struct Rc<
318  | |     T: ?Sized,
319  | |     #[unstable(feature = "allocator_api", issue = "32838")] A: Allocator = Global,
320  | | > {
     | |_- function or associated item `allocator` not found for this struct
...
1382 |           let alloc = unsafe { ptr::read(Self::allocator(&this)) };
     |                                                ^^^^^^^^^ function or associated item cannot be called on `Rc<T, A>` due to unsatisfied trait bounds
     |
note: if you're trying to build a new `Rc<T, A>` consider using one of the following associated functions:
      Rc::<T, A>::from_inner_in
      Rc::<T, A>::from_ptr_in
      Rc::<T, A>::new_in
      Rc::<T, A>::try_new_in
    --> library/alloc/src/rc.rs:375:5
     |
     |
375  |     unsafe fn from_inner_in(ptr: NonNull<RcBox<T>>, alloc: A) -> Self {
...
...
380  |     unsafe fn from_ptr_in(ptr: *mut RcBox<T>, alloc: A) -> Self {
...
...
694  |     pub fn new_in(value: T, alloc: A) -> Rc<T, A> {
...
...
797  |     pub fn try_new_in(value: T, alloc: A) -> Result<Self, AllocError> {
     |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: trait bound `T: Sized` was not satisfied
     |
     |
669  | impl<T, A: Allocator> Rc<T, A> {
     |      |
     |      unsatisfied trait bound introduced here
     |      unsatisfied trait bound introduced here
help: consider relaxing the type parameter's implicit `Sized` bound
     |
669  | impl<T: ?Sized, A: Allocator> Rc<T, A> {


error[E0599]: no method named `allocator` found for struct `ManuallyDrop<rc::Weak<T, A>>` in the current scope
     |
3064 |         let alloc = unsafe { ptr::read(this.allocator()) };
3064 |         let alloc = unsafe { ptr::read(this.allocator()) };
     |                                             ^^^^^^^^^ method not found in `ManuallyDrop<Weak<T, A>>`

error[E0599]: the function or associated item `allocator` exists for struct `Arc<T, A>`, but its trait bounds were not satisfied
     |
248  | / pub struct Arc<
248  | / pub struct Arc<
249  | |     T: ?Sized,
250  | |     #[unstable(feature = "allocator_api", issue = "32838")] A: Allocator = Global,
251  | | > {
     | |_- function or associated item `allocator` not found for this struct
...
1523 |           let alloc = unsafe { ptr::read(Self::allocator(&this)) };
     |                                                ^^^^^^^^^ function or associated item cannot be called on `Arc<T, A>` due to unsatisfied trait bounds
     |
note: if you're trying to build a new `Arc<T, A>` consider using one of the following associated functions:
      Arc::<T, A>::from_inner_in
      Arc::<T, A>::from_ptr_in
      Arc::<T, A>::new_in
      Arc::<T, A>::try_new_in
    --> library/alloc/src/sync.rs:289:5
     |
     |
289  |     unsafe fn from_inner_in(ptr: NonNull<ArcInner<T>>, alloc: A) -> Self {
...
...
294  |     unsafe fn from_ptr_in(ptr: *mut ArcInner<T>, alloc: A) -> Self {
...
...
711  |     pub fn new_in(data: T, alloc: A) -> Arc<T, A> {
...
...
844  |     pub fn try_new_in(data: T, alloc: A) -> Result<Arc<T, A>, AllocError> {
     |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: trait bound `T: Sized` was not satisfied
     |
     |
685  | impl<T, A: Allocator> Arc<T, A> {
     |      |
     |      unsatisfied trait bound introduced here
     |      unsatisfied trait bound introduced here
help: consider relaxing the type parameter's implicit `Sized` bound
     |
685  | impl<T: ?Sized, A: Allocator> Arc<T, A> {

error[E0599]: no function or associated item named `allocator` found for struct `sync::Weak` in the current scope
    --> library/alloc/src/sync.rs:2806:46
     |
     |
321  | / pub struct Weak<
322  | |     T: ?Sized,
323  | |     #[unstable(feature = "allocator_api", issue = "32838")] A: Allocator = Global,
324  | | > {
     | |_- function or associated item `allocator` not found for this struct
...
2806 |           let alloc = unsafe { ptr::read(Self::allocator(&this)) };
     |                                                ^^^^^^^^^ function or associated item not found in `Weak<T, A>`
     |
note: if you're trying to build a new `sync::Weak<T, A>` consider using one of the following associated functions:
      sync::Weak::<T, A>::new_in
      sync::Weak::<T, A>::from_raw_in
     |
     |
2627 |     pub fn new_in(alloc: A) -> Weak<T, A> {
...
...
2854 |     pub unsafe fn from_raw_in(ptr: *const T, alloc: A) -> Self {

For more information about this error, try `rustc --explain E0599`.
error: could not compile `alloc` (lib) due to 4 previous errors
warning: build failed, waiting for other jobs to finish...

rust-log-analyzer avatar May 13 '24 23:05 rust-log-analyzer

:facepalm: Oops, this depends on #124980, which isn't merged yet. Converted to draft, sorry for the noise.

zachs18 avatar May 13 '24 23:05 zachs18

@rustbot author

zachs18 avatar May 14 '24 00:05 zachs18

Replaced uses of fn allocator (which doesn't exist yet) with directly accessing the alloc field. No longer blocked on #124980.

@rustbot ready

zachs18 avatar May 17 '24 03:05 zachs18

@bors r+

Mark-Simulacrum avatar May 19 '24 23:05 Mark-Simulacrum

:pushpin: Commit c895f6e958ee34f6c52b612156e70465d18bd81a has been approved by Mark-Simulacrum

It is now in the queue for this repository.

bors avatar May 19 '24 23:05 bors