Implement placement-in protocol for Vec

[apasel422]

Open questions:

• Is BackPlace the right name for the struct?
• Should &mut Vec implement Placer? Or should it provide a back_place method like LinkedList does instead?

CC #30172, @rust-lang/libs, @pnkfelix

[rust-highfive]

r? @brson

(rust_highfive has picked a reviewer for you, use r? to override)

[petrochenkov]

I'd avoid using placement in stable push before doing proper performance measurements.

[nagisa]

I had some benchmarks implemented in nagisa@7b1b36d but they’re not enough at all to properly test all the cases (e.g. large-sized T).

[alexcrichton]

Nice! I'd be curious to discuss our conventions a little more here as well. It seems like we may want to both implement the placer on the types directly as well as have methods, but then we've got naming issues etc, etc. Fun bikesheds!

[aturon]

Yes, I think we should discuss the overall conventions here before going too much farther down that road -- let's be sure to cover it in the next libs meeting if we don't reach a resolution on this thread.

[bitshifter]

I'm very much a fly on the wall with this change. I was wondering if there's a precedent for an operator pushing into a container elsewhere? It looks a bit strange to me.

[petrochenkov]

I think we should discuss the overall conventions here before going too much farther down that road

The collection library conventions were specifically audited before 1.0. How on earth Vec ended up with last() as a way to obtain the last element and VecDeque/LinkedList with back()?
(Anyway, I still suggest the naming scheme mentioned in #31696 (comment))

[petrochenkov]

Also, the PR name is a bit misleading, v.place(i) <- elem - the most interesting (implementation- and performance-testing-wise) part of placement protocol for Vec - is not implemented.

[nagisa]

@aturon when discussing the feature please keep in mind the following point:

This is unstable and implementation as it is currently is mostly done for everybody to try the feature and see what works and doesn’t. Notably the “Should &mut Vec implement Placer? Or should it provide a back_place method like LinkedList does instead?” should eventually answer itself through use – if people find &mut Vec <- something not pleasant to use, we would change to .back_place method before stabilisation.

I was wondering if there's a precedent for an operator pushing into a container elsewhere? It looks a bit strange to me.

This is not an operator for pushing into containers; This is an operator for directly placing things into certain places that are not necessarily on the stack. Feature-wise the closest equivalent to this is C++’s emplacement, which, one could argue, hasn’t much special syntax.

[bitshifter]

This is not an operator for pushing into containers; This is an operator for directly placing things into certain places that are not necessarily on the stack. Feature-wise the closest equivalent to this is C++’s emplacement, which, one could argue, hasn’t much special syntax.

From the example for this PR:

let mut vec = vec![1, 2];
&mut vec <- 3;
&mut vec <- 4;
assert_eq!(&vec, &[1, 2, 3, 4]);

Which is effectively the same as C++11 std::vector emplace_back() - maybe "push" isn't the right description? Placement or not, adding a new element to the end of the vector which sounds like a push operation to me.

[bluss]

Is there any way we can avoid this becoming a de facto deprecation of the Vec::push() method? I mean, if placement in is always better, it's going to be the preferred style?

[hanna-kruppe]

@bluss It's always better when it works, but there are some theoretical circumstances where it doesn't work, e.g.:

&mut vec <- vec[0].clone();

More generally placement won't work if creating the value to be placed requires borrowing something also borrowed by the place (unless of course if none of the borrows is &mut). You can evaluate the value beforehand, of course, but this defeats the purpose of the placement syntax.

However, I'm not sure whether this will be common enough to warrant keeping push around. The let value = vec[0].clone(); &mut vec <- value; dance shouldn't have worse performance than push, so it comes down to convenience vs. nudging people into the direction of faster code.

[petrochenkov]

When emplacement roll-back is expensive and happens often, push is also preferable:

vec.place(middle) <- if flip_coin() == HEAD { 1 } else { return };

Before flipping the coin vector shifts half of its elements to the right to prepare the scratch space for the new element in the position i. On return vector has to recover its initial state and shift these elements back.

[alexcrichton]

Thanks for the PR @apasel422! The libs team discussed this during triage yesterday, and the conclusion was that we probably want to hold off with more in-place implementations if we can. We're slightly uncomfortable that LinkedList already implements this, and we'd like to hold off on more if possible.

Specifically, especially as the discussion here indicates, we likely need to lay out some conventions for naming, usage, what implements Placer, etc. T. This should also manifest itself in the form of an RFC (to foster discussion at large), so in light of that I'm going to close this for now.

@aturon has showed interest in helping shepherd a conventions RFC in this regard, as well. @apasel422 would you be interested in writing such an RFC?

[apasel422]

@alexcrichton An RFC is totally reasonable for this (I'll start thinking about writing one in the next couple of days), but I am curious as to why #31696 was accepted.

[alexcrichton]

Yeah we concluded it was probably premature for us to accept #31696 unfortunately :(

Thanks though!

[petrochenkov]

This is a very strange decision, emplacement needs implementation experience and performance measurements first of all, naming scheme is not important at the moment.

[aturon]

@petrochenkov @nagisa You both make reasonable points about wanting to experiment quickly and not being too cautious here. But, on the other hand, we'd prefer not to rapidly get into a situation where we have suboptimal de facto conventions or inconsistency across std -- which is what happened a lot prior to a dedicated API conventions process prior to 1.0.

(Personally, I think the names that landed for LinkedList should be improved, and I'd rather that not accidentally become a difficult to change convention just because we didn't want to take the time to think about it.)

This doesn't need to be heavyweight -- the libs team would just like to see a brief writeup of some basic naming/impl scheme. We can move quickly on it.

[alexcrichton]

@apasel422 the libs team discussed this recently and our conclusion was that we have some concrete ideas of how to revive this movement of implementations of these traits on collections. Would you still be interested in tackling this?

[alexcrichton]

Ah and to be more concrete (and to write down the notes), our conclusion was:

• Collections like Vec which have a clear "default place" can implement the protocol directly, e.g. vec <- item.
• Otherwise standardizing around the convention collection.place_location() <- item seems reasonable. For example vec.place_front() <- item

[apasel422]

@alexcrichton Sure. I'll investigate resurrecting this later today.

[alexcrichton]

Awesome! We're also thinking that we could turn this into a tracking issue with all collections listed so we could keep up with the progress. Maybe you'd like to take a look first though and see where this would all need to be implemented?

[apasel422]

@alexcrichton I don't see how we can get syntax like

let mut vec = vec![1, 2, 3];
vec <- 4;

instead of

let mut vec = vec![1, 2, 3];
&mut vec <- 4;

as Placer::make_place takes self by value, and the placement-in syntax doesn't autoref.

[alexcrichton]

@apasel422 hm we were definitely not aware of that! We can perhaps push forward with place_* methods and handle that issue later, however.

[nagisa]

@apasel422 we could make auto-ref work by temoprarily changing desugaring from Placer::make_place(var) to var.make_place() and implemeting perma-unstable impl Vec { fn make_place(&mut var) { Placer::make_place(var) }.

Either way we have rust-lang/rfcs#1286.