Skip to content

Warn when using [foo; 0] where foo is a non-const expression that evaluates to a Drop type #79580

Description

@bstrie

In #74836 it was discovered that [foo; 0] will leak foo, causing observable effects if foo has 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:

struct Foo;

impl Drop for Foo {
    fn drop(&mut self) {
        println!("dropped!")
    }
}

const FOO: Foo = Foo;

fn main() {
    println!("0 drops follow this");
    [FOO; 0];
    println!("1 drops follow this");
    [FOO; 1];
    println!("2 drops follow this");
    [FOO; 2];
}

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 run FOO'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 impls Drop:

  • When N is greater than 2, this will produce an insurmountable compile-time error (the compiler requires Copy, which is incompatible with Drop).

  • 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.

Activity

  1. added
    A-lintsArea: Lints (warnings about flaws in source code) such as unused_mut.
    on Nov 30, 2020
  2. added
    A-const-evalArea: Constant evaluation, covers all const contexts (static, const fn, ...)
    A-destructorsArea: Destructors (`Drop`, …)
    T-langRelevant to the language team
    on Nov 30, 2020
  3. RalfJung commented on Dec 1, 2020

    @RalfJung
    Member

    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.

  4. bstrie commented on Dec 1, 2020

    @bstrie
    ContributorAuthor

    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 [].

  5. RalfJung commented on Dec 2, 2020

    @RalfJung
    Member

    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 ;0 case 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!

  6. JakobDegen commented on Aug 31, 2022

    @JakobDegen
    Contributor

    Could someone close this? The underlying bug with [foo; 0] not dropping has been fixed nevermind, I misunderstood the issue. This isn't asking what I thought it was

  7. RalfJung commented on Aug 31, 2022

    @RalfJung
    Member

    @JakobDegen sure, but we should leave a link to the PR that fixed it -- do you have a PR number?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-ZSTArea: Zero-sized types (ZSTs).A-arrayArea: `[T; N]`A-const-evalArea: Constant evaluation, covers all const contexts (static, const fn, ...)A-destructorsArea: Destructors (`Drop`, …)A-lintsArea: Lints (warnings about flaws in source code) such as unused_mut.T-langRelevant to the language team

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions