RFC 2203 does not work for Vec::new

[ecstatic-morse]

RFC 2203 specifically calls out Vec::new as a motivating example, however this does not work on the current nightly.

fn main() {
    let _: [Vec<i32>; 4] = [Vec::new(); 4];
}

There are two reasons for this. First, promotion in array repeat expressions currently uses the "implicit" rules for promotability, which forbids promotion of function calls without #[rustc_promotable]. Second, the result of Vec::new() is marked as NeedsDrop since Vec<T> is Drop, which disqualifies it from promotion, even in an explicit context.

cc #49147

[Centril]

cc @eddyb @RalfJung @oli-obk @nikomatsakis

[eddyb]

The NeedsDrop thing (maybe) sounds like a bug in the old promotion logic.
The new logic in #63812 doesn't look at NeedsDrop at all, except for Candidate::Ref.

[ecstatic-morse]

A comment in qualify_consts:

// Any qualifications, except HasMutInterior (see above), disqualify
// from promotion.
// This is, in particular, the "implicit promotion" version of
// the check making sure that we don't run drop glue during const-eval.

I think explicit (non-Candidate::Ref) promotion contexts should indeed promote values with NeedsDrop, which is implied by the second half of the comment, but not the way things work currently.

If everyone is comfortable with this, I will add it to the rules for promotability.

[RalfJung]

@Centril or @nikomatsakis mentioned that we might at some point even allow using run-time expressions for array repeats... in the light of that it might not be a good idea to use the "explicit" rules for promotability. "explicit" is only okay if code would fail to compile when not promoted.

[ecstatic-morse]

If the type of the array element is Copy, failure to promote the initializer does not imply the code will fail to compile, so I think this argument applies to the current situation as well. That said, I don't know if we have to be quite that strict when deciding to use the explicit rules. We don't need to extend the lifetime of promoted expressions like we do for rvalue-static promotion.

[RalfJung]

If the type of the array element is Copy, failure to promote the initializer does not imply the code will fail to compile

Wait, what? I was under the impression that we first check Copy and only if that fails, force promotion. For us to use explicit promotion rules, failure to promote must imply failure to compile.

That said, I don't know if we have to be quite that strict when deciding to use the explicit rules. We don't need to extend the lifetime of promoted expressions like we do for rvalue-static promotion.

I think we do. The reason we are so strict is that code might do things (in particular, code inside const fn in other crates) that fails during CTFE but would succeed during run-time -- there might be "unconst" operations and they might occur only for certain arguments. These are things like observing the base address of a pointer, or using transmutes/unions to get around CTFE limitations. If that happens, that's bad enough, but we do not want that to lead to a compilation failure for a crate that could have compiled fine had we not promoted.

So, the strict stanza "failure to promote must imply failure to compile" has nothing to do with lifetimes or drop or so. It's all about not breaking people's ability to compile their code.

[ecstatic-morse]

Wait, what? I was under the impression that we first check Copy and only if that fails, force promotion. For us to use explicit promotion rules, failure to promote must imply failure to compile.

For non-Copy arrays, failure to promote does imply failure to compile. From this perspective, for Copy arrays, promotion is not involved at all. If we were to make array initialization occur unconditionally at runtime, promotion would not be involved ever. In this scenario as well as the status quo, we fail to compile if we ever try to promote the initializer and fail, so I think the requirements for explicit promotion are met here. The fact that we only sometimes try to promote the initializer makes this weird.

If you're not convinced by the above (I'm not entirely convinced either), we should switch to the following rules for promotability and make Vec::new #[rustc_promotable]:

Promotion Context	Allows Drop	Allows const fn calls
&expr	❌	❌
[expr; 4]	✔️	❌
simd_shuffle(expr)	❔	✔️

I don't think it matters whether we allow Drop for #[rustc_args_required_const], so we might as well forbid it. This would mean that every promotion context has slightly different rules around promotability, so the "implicit"/"explicit" distinction would no longer be enough.

[eddyb]

From this perspective, for Copy arrays, promotion is not involved at all.

Did we fix this though? I forget if we actually added that check.

[ecstatic-morse]

@eddyb You're correct. On nightly, all array initializers are promoted if the feature is enabled (look at the generated MIR).

[eddyb]

This is what I want that table to look like btw, if not obvious:

Promotion Context	Allows Drop	Allows const fn calls
&expr	❌	❌
[expr; 4]	✔️	✔️
simd_shuffle(expr)	✔️	✔️

The const fn question is trickier, but the Drop one is based on the fact &expr doesn't move expr but the other two promoted forms do move it.

Moving expr means that there can't be a Drop terminator that would've ran at runtime, but promotion would remove, unlike borrowing expr where that is very much a concern.

[ecstatic-morse]

While working on this, I've become a bit more concerned about allowing NeedsDrop values (i.e. Vec::new(), not Option::<Box<i32>>::None). What would happen if Box::new were ever to become a const fn? I don't think we can allow [Box::new(4); 2], since most reasonable implementations would create two Boxes pointing to the same underlying allocation. Changing the semantics so that the expression in the initializer is executed once for each spot in the array would change the meaning of code like [rand(); 2], which results in an array where all values are equal.

[eddyb]

It's the same as const X = Box::new(4); [X, X] - we wouldn't allow that either, and it wouldn't be based on the presence of Drop.

[ecstatic-morse]

What differentiates that code from the version with `Vec` replacing `Box` though? How can we justify one without the other?

…

On Thu, Oct 24, 2019, 22:16 Eduard-Mihai Burtescu ***@***.***> wrote: It's the same as const X = Box::new(4); [X, X] - we wouldn't allow that either, and it wouldn't be based on the presence of Drop. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#65737?email_source=notifications&email_token=AHAZGRB5YYSBQ7ZDXFY4ZQLQQJ6MTA5CNFSM4JEJQIUKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECHGS2A#issuecomment-546204008>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHAZGRFOXQ4D5WIKDRFV2H3QQJ6MTANCNFSM4JEJQIUA> .

[eddyb]

I don't know, to be honest. That's a question for @oli-obk I guess.

EDIT: actually we probably want to allow copying Box::new(()) since it doesn't allocate, so that's going to be fun.

We can make [vec![]; 2] work without a const fn call, and since it would be an inherent associated const, we can peek at the definition to confirm no allocations.
Kind of makes me wish Vec::new() didn't exist.

[oli-obk]

Please direct all your const heap related questions to rust-lang/const-eval#20

It is not a problem, because Box::new(42) may never end up in a final constant as per the linked issue above. So [Box::new(42); 2] is not ok, but [vec![]; 2] and [Vec::new(); 2] are. The reason is twofold:

1. if there are no heap allocations, the value may be in a constant as per today's rules
2. if there are heap allocations, only ConstSafe types may be in constants, and String, Vec and Box are most definitely not ConstSafe. You can put them behind a reference though, because they are ConstRefSafe, and &ConstRefSafe: ConstSafe.

So you can do [&Box::new(42); 2], but not [Box::new(42); 2]

[eddyb]

@oli-obk I think you've fallen again in the trap of assuming no polymorphism.
I don't see how you can tell whether allocations exist without looking at the final value (or using symbolic evaluation? but that can't work for associated consts)

[oli-obk]

Well... we could still allow it for all constants where we can compute a final value, but maybe that's not a good idea

[RalfJung]

For non-Copy arrays, failure to promote does imply failure to compile. From this perspective, for Copy arrays, promotion is not involved at all.

Oh I see, I misread.

If we were to make array initialization occur unconditionally at runtime, promotion would not be involved ever.

What were you assuming this mechanism would look like?

The fact that we only sometimes try to promote the initializer makes this weird.

Agreed.
I'd be much more comfortable requiring people to write [const { Vec::new() }; n] as in our floating proposal for explicit promotion.

make Vec::new #[rustc_promotable]

No please not. My long-term goal is to remove rustc_promotable slowly but steadily over many years, starting the moment we have implicit promotion. We shouldn't add this attribute to more functions, we should remove it from existing functions if it looks like nobody uses it.
(Or at least, at some point we should have an edition that doesn't implicitly promote any const fn calls.)

[RalfJung]

The feature const_in_array_repeat_expressions is gone, the intention is to use inline consts instead.

This works, but needs a type annotation:

#![feature(inline_const)]
fn main() {
    let _: [Vec<i32>; 4] = [const { Vec::<i32>::new() }; 4];
}

I think this means we can close this issue?