RFC: Constants in array repeat expressions

[Centril]

Rendered.
Tracking issue

Relaxes the rules for repeat expressions, [x; N] such that x may also be const, in addition to typeof(x): Copy. The result of [x; N] where x is const is itself also const.

[durka]

+1 from me. This looks like a small and intuitive change. I really like the application to array initializers.

[eddyb]

See also #1169.

[ExpHP]

I would rather see T: Clone.

[Centril]

@ExpHP Are they mutually exclusive? Can't both be done?

[ExpHP]

@Centril all I can really say is I hope they are compatible!

(I mean, who doesn't want to have their 🍰 and eat it too?)

[Centril]

@ExpHP A workable rule for having your 🍰 and eating it too would be:

1. Assume a predicate is_const : Value -> {T, F}.
2. Assume [x; N] where is_const(N) = T.
3. Pick a method
   1. If is_const(x) then the method of this RFC is used.
   2. Else if typeof(x): Copy then the method used currently is used.
   3. Else if typeof(x): Clone then a future method is used.
   4. Else a type error is raised.

Thus, this RFC should be forward compatible with Clone (which I agree should if possible be supported).

[ExpHP]

Hm, I'm not sure that the solution is that simple. That creates a potentially confusing scenario where two different expressions that evaluate to the same value could have different semantics when they appear inside an array repetition, as one could get cloned which may have side-effects.

---

On the other hand, upon reflection I'm not sure how attached I actually am to T: Clone. I think it's quite likely that many of the problems where I wished that it supported T: Clone were cases like [None; n] and [vec![]; n], where const expressions would also have helped.

[ExpHP]

A compelling argument from @Centril on IRC is that T: Clone can be provided by a library function, while support for constexprs clearly requires language support.

[Centril]

(summarizing what we discussed on IRC):

As you put it I think [None; n] and [Vec::new(); n] are some of the more common examples, which this RFC enables (assuming Vec::new() becomes const).

If there is confusion as to whether the const or Clone method is used then you can always have a dedicated method for Clone. Or even: (straw-man function name) make_array(42, |index| elem_from_index(index)) which would enable: make_array(42, |_| val.clone()).

I also think Clone implementations that have other side effects than touching the heap and modifying some reference count are quite rare. In those cases, I think the developer knows what they are doing.

[burdges]

Right now, you can do this with ArrayVec without all the restrictions, but..

We should get impl<A, const N: usize> FromIterator<A> for [A; N] eventually, right? It does not exist for N in 0..32 now, but afaik no obstacles exist, right?

[Centril]

@burdges You'd have to decide what to do when the iterator has less - panic seems the (only) reasonable option... But sure, it should be doable eventually =)

[burdges]

Yes, you might want impl<A, const N: usize> FromIterator<A> for Option<[A; N]> or maybe some Result variant. At that point, you can write simply let a: [T; N] = repeat(x).collect().unwrap(); or add in .map(|_| { ... }) for whatever you like.

[Centril]

@burdges Right. Do note that the result of such an expression is non-const unlike the result of [x; N] where is_const(x). But sure, such an impls should be added also.

[eddyb]

Note for future self/implementers: moving the Copy check to borrowck is necessary to avoid is_const on the HIR. Current MIR for [e; N] is tmp = e; dst = [tmp; N] and so the current rule could just be "tmp used more than once" in MIR borrowck, which means the implementation involves just doing rvalue promotion on tmp.

That makes Clone non-viable, which, frankly, I'm very happy with anyway.

[nikomatsakis]

@eddyb can you say a bit about how we would implement this? I haven't though deeply here but I am mildly worried that it will be hard for us to test if an expression "is const" -- i.e., do we just have to like run it through miri and see what happens?

Put another way: it's one way to have a set of expressions that must be const and then check that they are.

It's another to have something that might or might not be const and to impose different requirements depending on the result. Seems like it could interact poorly with type inference etc.

Otherwise, I like the idea.

[eddyb]

@nikomatsakis No, we'd just run a special case for rvalue promotion, just like we do for borrowed temporaries and for SIMD shuffle indices.

[nikomatsakis]

@eddyb

we'd just run a special case for rvalue promotion, just like we do for borrowed temporaries and for SIMD shuffle indices.

OK, so we'd be defining this not really for const expresions per se but for 'promotable expressions', which is (I think) not the same set, right? That is, we do some analysis to decide if something is promotable, and it is presumably a bit conservative, or could at least in theory be.

Still, it seems like this case will be mildly harder to handle. For example, we can do rvalue promotion on the MIR level before borrow check runs, and hence borrowed temporaries having a longer lifetime just "falls out", but we can't do the same here -- well, I suppose we could wait to check the requirement that T: Copy during borrow-check. (That actually almost makes sense; you can think of the MIR as doing copies internally, and hence imposing the requirement T: Copy.)

I am basically hoping we can avoid having to know whether something is constant during type check but instead have that be computed from the MIR alone.

[eddyb]

I am basically hoping we can avoid having to know whether something is constant during type check but instead have that be computed from the MIR alone.

That's what I was saying in #2203 (comment) ("necessary to avoid is_const on the HIR"), except on MIR it wouldn't be is_const but is_promotable, at least to start with.

For example, we can do rvalue promotion on the MIR level before borrow check runs, and hence borrowed temporaries having a longer lifetime just "falls out", but we can't do the same here -- well, I suppose we could wait to check the requirement that T: Copy during borrow-check.

That last part is what I was saying ("so the current rule could just be "tmp used more than once" in MIR borrowck"), but instead of T: Copy specifically, we treat [operand; n] as evaluating operand:

• never, if n == 0
• one time, if n == 1
• twice/many times, if n > 1

In the n > 1 case, if operand is Copy or Const, it'd always work, but if it's Move, it'd always cause an error - hence replicating T: Copy in a more move-checking-oriented manner.

I don't understand the part about "a longer lifetime", but I hope the above explanation suffices.

[Centril]

@nikomatsakis: I've updated and aligned the RFC according to @eddyb's ideas.. Hopefully that addresses your concerns =)

[nikomatsakis]

I think @eddyb and I are aligned here in terms of how we want this to work in the compiler, so if they are satisfied I think I am. And reading over their comments, it makes sense.

[nikomatsakis]

@rfcbot fcp merge

I propose that we accept this RFC. The RFC expands [expr; N] expressions such that expr must be either a constant or a value of type Copy. More specifically, the expression must either be the sort of expression that we would already be giving 'static lifetime through the existing rvalue promotion rules, as @eddyb and I discussed here. (This mollifies my main concerns around implementability: the idea is roughly that the borrow checker would be responsible for enforcing the constraint on the type of this value, and it runs after constant promotion and hence can see whether the expression is a constant.)

[rfcbot]

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

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

No concerns currently listed.

Once a majority of reviewers approve (and none object), 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.

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[rfcbot]

The final comment period is now complete.

[Centril]

Huzzah! The RFC has been accepted.

Tracking issue: rust-lang/rust#49147