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
a
is destroyed oncefoo
returns, so we would have to run the destructor whenfoo
returns, _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
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)
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
foo
returns anda
goes out of scope, because the value ina
has been moved. If we did not have array literal syntax and used a constructor instead, the value ina
would 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.