This bug was reported on StackOverflow today. Filipe Rodrigues noticed that the initializer of a zero-length array is leaked:
#[derive(PartialEq, Debug)]
struct A;
impl Drop for A {
fn drop(&mut self) {
println!("Dropping A");
}
}
fn main() {
let vec: Vec<A> = vec![];
let a = A;
assert_eq!(vec, [a; 0]);
}
The code above does not run drop() for a, while I'd expect that it should. According to a comment by @mcarton, this regression occurred between Rust 1.11.0 and 1.12.0, i.e. when MIR code generation was enabled by default.
Minimized:
struct A;
impl Drop for A {
fn drop(&mut self) {
println!("Dropping A");
}
}
fn main() {
let a = A;
let _ = [a; 0];
}
Marked as p-low as per the discussion
This is a really weird corner case. If we modify the example slightly:
struct A;
impl Drop for A {
fn drop(&mut self) {
println!("Dropping A");
}
}
fn foo() -> [A; 0] {
let a = A;
[a; 0]
}
fn main() {
foo();
}
them [a; 0] still moves a (a cannot be used again). However, the storage for a is destroyed once foo returns, so we would have to run the destructor when foo returns, not when [a; 0] is dropped. Effectively, the borrow-checker and the MIR drop logic disagree about who owns a.
Regardless of what we end up doing, I think we should add a lint for this kind of scenario, as I think whatever behavior we use is going to be surprising.
However, the storage for
ais destroyed oncefooreturns, so we would have to run the destructor whenfooreturns, _not_ when[a; 0]is dropped.
I would not expect a destructor to run when foo returns and a goes out of scope, because the value in a has been moved. If we did not have array literal syntax and used a constructor instead, the value in a would clearly be moved in the call and its ownership transferred to the constructor:
fn foo() -> [A; 0] {
let a = A;
<[A; 0]>::new(a)
}
When the array length is zero the repeating array "constructor" happens not to use the repeated value so it should be responsible for dropping it.
When the array length is zero the repeating array "constructor" happens not to use the repeated value so it should be responsible for dropping it.
Agreed. Dropping should happen at the location of [expr; 0].
I am not sure a lint is the right solution here? If we want to change MIR building (this looks like a clear bug to me), I do not see what a lint would achieve.
Hm, why does this even build? Usually array initializers only work for Copy types.
Is there a special case for when the array length is <= 1? I guess then we also need a special case during drop elaboration (or so) for when the array length is exactly 0?
Cc @matthewjasper @nikomatsakis (not sure who else to ping for MIR stuff)
Hm, why does this even build? Usually array initializers only work for
Copytypes.Is there a special case for when the array length is <= 1? I guess then we also need a special case during drop elaboration (or so) for when the array length is exactly 0?
Cc @matthewjasper @nikomatsakis (not sure who else to ping for MIR stuff)
Yup, it appears that [value; 1] is allowed to simply move a non-copy into an array: playground. Definitely a bit odd but I can certainly understand why this'd be the case.
Most helpful comment
I would not expect a destructor to run when
fooreturns andagoes out of scope, because the value inahas been moved. If we did not have array literal syntax and used a constructor instead, the value inawould clearly be moved in the call and its ownership transferred to the constructor:When the array length is zero the repeating array "constructor" happens not to use the repeated value so it should be responsible for dropping it.