Placement in/box refinement

[Stebalien]

Supersedes #1401

rendered

[eddyb]

I almost feel like Unique<T, A> should be Place<T, A>'s "filled" counterpart, if both were structs and A described how the allocation should be handled.

[eddyb]

To expand on the special HIR node idea:

PLACE <- EXPR // gets desugared into:
ExprEmplace(Placer, PLACE, EXPR)

box EXPR // gets desugared into:
ExprEmplace(Boxer, ExprCall(Default::default, []), EXPR)

This way, we could guarantee that 0 unnecessary copies are being made, DST support can be added without having to come up with impossible source desugarings and, probably the best part, we can employ a simpler form of rust-lang/rust#27292 only for ExprEmplace:

If we have an expected type, i.e. ExprEmplace(Placer, _, E): P<A, B, C>, we check that P<A, B, C>: Placer<T>.
In case we do get a match, T: Sized must hold and we can use T as the expected type for E.

Otherwise, it's either a legitimate error, or our P<A, B, C> can only be produced by unsizing the result of ExprEmplace (e.g. Box<Trait>). We can assume it's not an error and attempt to make it compile.

Thankfully, the unsizing coercions relevant here only involve structs (and Box, which we can treat as a struct).
This simplifies our job greatly and if more types support unsizing coercions in the future, we can simply choose not to handle them here.

We can take the most conservative stance and replace all type params in P<A, B, C> with inference variables to get a condidate for our missing intermediary smart pointer, e.g. P<X, Y, Z>.

Then, we require that P<X, Y, Z>: CoerceUnsized<P<A, B, C>> + Placer<T>.
This will link up all the type params and allow us to type-check the emplace expression.

E.g. for Box<Trait, A>, we require Box<X, Y>: CoerceUnsized<Box<Trait, A>> + Boxer<T> which will result in X = T, Y = A and a T: Trait obligation.
In the end we get our intermediary smart pointer, Box<T, A>, with a Boxer impl we can get the associated Place from, and a coercion to Box<Trait, A>.

Unlike rust-lang/rust#27292, this also works if you replace Trait with [T], and does not affect coercions in existing stable code.

[mahkoh]

Why would the Allocator be constructed in the desugaring and the Boxer have an Allocator parameter? One can simply

impl<T, A> Boxer<T> for XYZ<T, A> where A: Allocator+Default

Having the Boxer trait know anything about allocators is completely unnecessary and a huge restriction.

[mahkoh]

@eddyb Doesn't move_val_init already do what you want? Except that move_val_init doesn't unconditionally get translated to a memcpy for some reason.

[eddyb]

@mahkoh The problem is that you can't desugar to move_val_init(ptr, EXPR) because it requires unsafe (and you don't want to allow unsafe code in EXPR) and it has to be its own statement in a nested block so coercion propagation is pretty much screwed.

[Stebalien]

@mahkoh, how is this a restriction? Boxer takes an allocator to allow us to define the following blanket impl:

impl<T, D, B> Placer<D, B> for T
   where T: Allocator,
          B: Boxer<D, T>
{
    type Place = B::Place;

    fn make_place(self) -> Self::Place
        where Self: Sized
    {
        B::make_place(self)
    }
}

Which, in turn, lets us do the following:

impl<T, A> Boxer<T, A> for Box<T, A> where A: Allocator { /* ... */}

let boxed: Box<_> = CustomAllocator::new(options) <- thing;

[mahkoh]

The Boxer implementation can acquire an allocator (if it even needs one) by means other than Default.

[Stebalien]

@mahkoh, sorry, hit enter too soon. See my updated post.

[mahkoh]

@Stebalien All of this can be done without the restrictions imposed by having Boxer know about allocators. E.g.

let a: Box<_, A1> = box val; // where A1: Default

let a = Box::with_pool(CustomAllocator::new(options)) <- thing;

[mahkoh]

The first line is equivalent to

let a = Box::with_pool(A1::default()) <- val;

[Stebalien]

@mahkoh, which means that one has to implement a new Placer. Regardless, I still don't see how this is a restriction. What is the use case for using box value without an Allocator? Note, the proposed system still supports your syntax.

[mahkoh]

Note, the proposed system still supports your syntax regardless.

It doesn't support box without a Default allocator.

What is the use case for using box value without an Allocator?

The object might want to use a globally shared allocator stored in a static variable or might want to construct an allocator with custom options.

[mahkoh]

which means that one has to implement a new Placer.

And where is the problem with that?

[mahkoh]

You also don't need a "new" Placer. See the following pseudocode

struct Box<T: ?Sized, A>
    where A: Allocator,
{
    alloc: A,
    val: *mut T,
}

struct BoxBuf<T, A>
    where A: Allocator,
{
    alloc: A,
    val: *mut T,
}

impl<T, A> Boxer<T> for Box<T, A>
    where A: Allocator+Default,
{
    type Place = BoxBuf<T, A>
    fn make_place() -> BoxBuf<T, A> {
        let mut alloc = A::default();
        /* ... */
        BoxBuf {
            alloc: alloc,
            val: val,
        }
    }
}

impl<T, A> Placer<T, Box<T, A>> for BoxBuf<T, A>
    where A: Allocator,
{
    type Place = Self;
    fn make_place(self) -> Self { self }
}

impl<T, A> Place<T> for BoxBuf<T, A>
    where A: Allocator,
{
    type Owner = Box<T, A>;
    fn pointer(&mut self) -> *mut T { self.val }
    unsafe fn finalize(self) -> Box<T, A> {
        Box {
            self.alloc,
            self.val,
        }
    }
}

It seems that what you actually want is a default implementation of Placer for all Place implementations.

impl<T, P> Placer<T, <P as Place<T>>::Owner> for P
    where P: Place<T>,
{
    type Place = Self;
    fn make_place(self) -> Self { self }
}

[mahkoh]

To be quite honest, I don't think there is a need for the Placer trait at all. The only use it has is that it can be implemented for Allocators so that you can use the HEAP <- val syntax to construct arbitrary containers.

This can also be done with the Place trait alone:

Box::with_pool(HEAP) <- val

So the only advantage seems to be the shorter syntax. The most common cases are

• the box val case if there is no need for error handling
• the try!(xyz) <- val case if there is a need for error handling

Neither of these cases is served by the Placer trait. There is also the problem with that Allocators have no idea how to construct containers and that the Placer implementation for Allocators is limited by this. This inversion then leaks into the placement traits by having them suddenly depend on Allocators (this RFC).

Apart from this forced dependency, allocators and placement traits are orthogonal.

[eddyb]

@mahkoh also consider vec <- x and hash_map.entry(k) <- v.

[mahkoh]

@eddyb The first one doesn't work because Placer consumes self. I'll think a bit about the second one.

[eddyb]

@mahkoh it would work if vec: &mut Vec<T> and/or <- does autoref on the LHS.

[mahkoh]

@eddyb Is there a reason that Place::pointer cannot do the heavy lifting? (Destroying the current entry in the hashmap if it exists, reserving a slot in the vector.)

[Stebalien]

@eddyb, note, as of this RFC, it has to be vec.back() <- x (unless we get auto referencing).

@mahkoh, FYI, it's PLACER <- thing;, not PLACE <- thing;.

Re your previous comments, this really just comes down to the common. With the system proposed in the RFC, one would write:

let boxed: Box<_> = HEAP <- value;
let object = Object::from_object_pool() <- value;

With your system, one would write:

let boxed: Box<_> = Box::with_allocator(HEAP) <- value;
let object: Object = box value;

I'll try to come up with a way to make both cases simpler.

[eddyb]

@mahkoh The primary concern @pnkfelix had, IIRC, is handling panics.

[mahkoh]

@eddyb I see. Well, I'm not bothered much by the Placer trait itself. It's the idea to implement it for Allocators that seems wrong.

[Stebalien]

@eddyb Is there a reason that Place::pointer cannot do the heavy lifting? (Destroying the current entry in the hashmap if it exists, reserving a slot in the vector.)

Also, Place::pointer takes self by reference so you'd have to handle the case of user code calling pointer more or less than once.

[eddyb]

@Stebalien Well that could be made unsafe and the compiler implementation of emplace can do the right thing (TM).

@mahkoh Another "nice" example: arena <- value where arena: &Arena.

[mahkoh]

@Stebalien

Also, Place::pointer takes self by reference so you'd have to handle the case of user code calling pointer more or less than once.

Just go full unsafe in all signatures. Does anyone expect that these traits will be used much outside of the expansions and possibly unsafe code?

FYI, it's PLACER <- thing;, not PLACE <- thing;.

I'm aware.

With your system, one would write:

In the first case you don't have to write the type on the lhs because Box::with_allocator already contains all the type information:

let boxed = Box::with_allocator(HEAP) <- value;

[nagisa]

s/Boxer’s/Boxers/?

[nagisa]

pop a stack frame

[Stebalien]

Fixed.

[nagisa]

All in all, everything related to Allocator looks hasty (since we do not have any design related to them landed yet) but the changes to the placement traits look okay overall.

[Stebalien]

@nagisa

All in all, everything related to Allocator looks hasty (since we do not have any design related to them landed yet) but the changes to the placement traits look okay overall.

Replying again here as my comment was swallowed in the push. IIRC, the original motivation of in (PLACE) box EXPR (or the new PLACE <- EXPR) was placing in a custom allocator so making sure it's compatible with that use case is important. Note: This RFC makes no assumptions about the definition of the Allocator trait, just that one exists. Regardless, I'm going to update the RFC to use the traits I proposed in #1426 (comment) which means that the allocator integration can be implemented later.

[Stebalien]

All (specifically, @nagisa),

I've updated the proposal to move the allocator related logic out of the Boxer into the Allocate trait and addressed the need for a Placer trait.

[pnkfelix]

typo: "implemeinting" --> "implementing"

[nikomatsakis]

Can someone summarize the state of this RFC with respect to the comment thread? Does the text include any desired updates? Are important "non-desired" updates summarized in the alternatives section? I'd like to make decisions on older RFCs like this one =)

[Ericson2314]

I recall there being good ideas here, but it will be a lot easier to evaluate once Allocators are implemented, so I suggest waiting for that.

[Stebalien]

There are also a few updates I want to make to take advantage of specialization. Should I close this for now and re-open it when allocators land (or are you planning on stabilizing placement new before the allocation API)?

[pnkfelix]

Yeah I haven't moved on landing allocators because I was waiting until the pieces were in place to support them on the library types (see RFC 1327)

But now the most fundamental language change for that has landed, so I hope to make forward progress there soon.

I don't think you need to close this RFC in the meantime.

[Ericson2314]

Yay!

[strega-nil]

@nrc @aturon @pnkfelix

This would be a good RFC for postponement, predicated on getting allocators.

[aturon]

@ubsan Agreed -- let's revisit this down the line, when more of the substrate has stabilized.

@rfcbot fcp postpone

[rfcbot]

Team member @aturon has proposed to postpone this. The next step is review by the rest of the tagged teams:

• @aturon
• @eddyb
• @nikomatsakis
• @nrc
• @pnkfelix
• @withoutboats

No concerns currently listed.

Once these reviewers reach consensus, 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.

[aturon]

OK, I'm going to close this as postponed. Work on placement has largely stalled; we should probably revisit the whole question at some point.