Rust: Initializing a zero-length array leaks the initializer

Created on 27 Jul 2020  路  8Comments  路  Source: rust-lang/rust

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.

A-destructors A-mir C-bug P-low T-compiler regression-from-stable-to-stable

Most helpful comment

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.

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.

All 8 comments

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 once foo returns, so we would have to run the destructor when foo 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.

Was this page helpful?
0 / 5 - 0 ratings