Repository navigation
Warn when using [foo; 0] where foo is a non-const expression that evaluates to a Drop type #79580
Description
Activity
- addedA-lintsArea: Lints (warnings about flaws in source code) such as unused_mut.Area: Lints (warnings about flaws in source code) such as unused_mut.
on Nov 30, 2020 - addedA-const-evalArea: Constant evaluation, covers all const contexts (static, const fn, ...)Area: Constant evaluation, covers all const contexts (static, const fn, ...)A-destructorsArea: Destructors (`Drop`, …)Area: Destructors (`Drop`, …)T-langRelevant to the language teamRelevant to the language team
on Nov 30, 2020 Specifically, it is considered correct that a constant array repeat expression [FOO; N] will run FOO's destructor N times.
FWIW, so far I think I am the only one who explicitly said that this is the desired behavior. I would not treat this as consensus yet, since the lang team has not made a statement on the subject.
When N is greater than 2, this will produce an insurmountable compile-time error (the compiler requires Copy, which is incompatible with Drop).
This is only part of the story. The compiler will then try to use promotion to turn this into an array repeat expression of a constant. However, promotion ensures that there is no destructor that needs running, so the conclusion remains the same.
On the subject of the lint itself, I have no strong opinion. However, I do feel rather strongly that #74836 should be fixed by dropping the affected value. We should first get our semantics right, then we can consider if we want to lint against
[expr; 0]. Once we did that I feel like the motivation for the lint is rather weak -- I have a hard time imagining how this would actually be a problem in practice.I would not treat this as consensus yet, since the lang team has not made a statement on the subject.
The lang team has just agreed to merge #79270, so as of today we can confidently call this the intended behavior.
However, promotion ensures that there is no destructor that needs running, so the conclusion remains the same.
I admit that I'm a bit uncertain regarding promotion rules; I see there's a document at https://github.com/rust-lang/const-eval/blob/master/promotion.md , but shouldn't something this semantically important have its own associated RFC and tracking issue?
I do feel rather strongly that #74836 should be fixed by dropping the affected value
Indeed, I don't think anyone's in disagreement about this. Non-const
[foo; 0]will run one destructor, and const[foo; 0]will run zero. It's the existence of this curious edge case that is sole motivation for this issue; notably, the goal is not to stop people from using zero-size arrays, but rather to encourage them to initialize them the idiomatic way via[].The lang team has just agreed to merge #79270, so as of today we can confidently call this the intended behavior.
The lang team has made no statement about the
;0case there. I have no idea where you are getting your confidence from.I admit that I'm a bit uncertain regarding promotion rules; I see there's a document at https://github.com/rust-lang/const-eval/blob/master/promotion.md , but shouldn't something this semantically important have its own associated RFC and tracking issue?
Promotion came to be with an RFC (well, back then implementation usually predated RFCs, but the RFC kind of describes the situation at the time, from all I know) and since then unfortunately was extended in an ad-hoc fashion without RFCs or really any kind of design. For more than a year now we are trying to document the status quo, and still we find new ways in which reality differs from the above document. For example, turns out what I said about drops above is wrong; that only applies to lifetime extension, not array repeat expressions. (But possibly it should.)
So yes, it should have its own associated RFC and whatnot. But sadly it doesn't, and I spent a lot of this year cleaning that up. We're getting somewhere though!
- addedA-ZSTArea: Zero-sized types (ZSTs).Area: Zero-sized types (ZSTs).
on Oct 6, 2021 Could someone close this? The underlying bug withnevermind, I misunderstood the issue. This isn't asking what I thought it was[foo; 0]not dropping has been fixed@JakobDegen sure, but we should leave a link to the PR that fixed it -- do you have a PR number?
In #74836 it was discovered that
[foo; 0]will leakfoo, causing observable effects iffoohas a destructor. In response #74857 was opened in order to warn users against using[foo; 0]to construct a zero-length array in favor of[], but this was deemed to be overkill.However, since then some new information has emerged that I believe calls for re-evaluating this decision. The problem comes from the use of array-repeat expressions in constant contexts. Consider the following program, which works on stable Rust today:
While the stabilization of array-repeat expressions on constant values was accidental (#79270), people appear to be in agreement that this behavior is correct: #79270 (comment) . Specifically, it is considered correct that a constant array repeat expression
[FOO; N]will runFOO's destructor N times.Consider how this compares to the use of array-repeat expressions in non-constant contexts. For some non-constant expression
[foo; N]where foo implsDrop:When N is greater than 2, this will produce an insurmountable compile-time error (the compiler requires
Copy, which is incompatible withDrop).When N is 1, this will have identical results to the const-context array-repeat expression, and is not a problem.
When N is 0, this will run the destructor exactly once, which differs from the behavior of const-context array-repeat expressions, which will not run any destructor. (Technically this is contingent on Initializing a zero-length array leaks the initializer #74836 being fixed, but everybody agrees that the current leaking behavior is incorrect).
In light of this divergence of behavior between const and non-const array-repeat expressions, it's worth discussing whether #74857 should be revived, especially since the fix is as simple as not using
[foo; 0]and simply using[]instead.