Thread overview
Struct field destructor not called when exception is thrown in the main struct destructor
5 days ago
tchaloupka
5 days ago
Ali Çehreli
5 days ago
Ali Çehreli
5 days ago
Paul Backus
5 days ago
Paul Backus
4 days ago
tchaloupka
5 days ago
Found a pretty nasty bug in vibe-d: https://github.com/vibe-d/vibe.d/issues/2484

And it's caused by this behavior.

```D
import std;

struct Foo {
    Bar bar;
    bool err;

    ~this() {
        // scope(failure) destroy(bar); // < this fixes the Bar destructor call
        enforce(!err, "Test err");
    }
}

struct Bar {
    static int refs;
    ~this() { refs--; }
}

void main()
{
    {
        Foo f;
        Bar.refs = 1;
    }
    assert(Bar.refs == 0);

    try () {
        Foo f;
        f.err = true;
        Bar.refs = 1;
    }();
    catch (Exception ex) {}
    assert(Bar.refs == 0);
}
```

So when the exception is thrown within Foo destructor (and it's bad on it's own but can easily happen as destructors aren't nothrow @nogc by default).

Is this behavior expected?
5 days ago
On 10/16/20 6:12 AM, tchaloupka wrote:

> struct Foo {
>      Bar bar;
>      bool err;
>
>      ~this() {
>          // scope(failure) destroy(bar); // < this fixes the Bar
> destructor call
>          enforce(!err, "Test err");

Well, that check means "cannot continue", which means the compiler stops executing the destruction code because it can't. (It would a serious bug if it continued execution in a state that the program knows to be invalid.)

>      }

Conceptually, bar's destructor is called on that closing brace but we decided to abort mission earlier. Your calling destroy(bar) may or may not be wrong in case of 'err'. Only you know at that point.

> Is this behavior expected?

Yes.

Ali

5 days ago
On Friday, 16 October 2020 at 13:12:04 UTC, tchaloupka wrote:
> So when the exception is thrown within Foo destructor (and it's bad on it's own but can easily happen as destructors aren't nothrow @nogc by default).
>
> Is this behavior expected?

This is a compiler/language bug. It was fixed in DMD 2.083.0, but the new behavior requires a -preview switch, since it has the potential to break code.

More information:

- https://dlang.org/changelog/2.083.0.html#reboot14246
- https://dlang.org/changelog/2.085.0.html#preview-flags
5 days ago
On Friday, 16 October 2020 at 15:19:51 UTC, Paul Backus wrote:
> On Friday, 16 October 2020 at 13:12:04 UTC, tchaloupka wrote:
>> So when the exception is thrown within Foo destructor (and it's bad on it's own but can easily happen as destructors aren't nothrow @nogc by default).
>>
>> Is this behavior expected?
>
> This is a compiler/language bug. It was fixed in DMD 2.083.0, but the new behavior requires a -preview switch, since it has the potential to break code.
>
> More information:
>
> - https://dlang.org/changelog/2.083.0.html#reboot14246
> - https://dlang.org/changelog/2.085.0.html#preview-flags

Oops, I misread the destructor as a constructor in the original post. Never mind!
5 days ago
On 10/16/20 9:12 AM, tchaloupka wrote:

> So when the exception is thrown within Foo destructor (and it's bad on it's own but can easily happen as destructors aren't nothrow @nogc by default).
> 
> Is this behavior expected?

I would say it's a bug. The compiler is going to call the member destructor even if the hand-written destructor does it too. If the compiler wants to take responsibility for cleaning up members, it should take full responsibility. In fact, there is no way to instruct the compiler "I'm handling the destruction of this member", so I don't see why it should matter if you exit the function via exception it should be any different.

-Steve
5 days ago
On 10/16/20 11:11 AM, Ali Çehreli wrote:
> On 10/16/20 6:12 AM, tchaloupka wrote:
> 
>  > struct Foo {
>  >      Bar bar;
>  >      bool err;
>  >
>  >      ~this() {
>  >          // scope(failure) destroy(bar); // < this fixes the Bar
>  > destructor call
>  >          enforce(!err, "Test err");
> 
> Well, that check means "cannot continue", which means the compiler stops executing the destruction code because it can't. (It would a serious bug if it continued execution in a state that the program knows to be invalid.)

The destruction of members is outside the destructor's purview. It can't turn the destruction off, so it should logically be considered part of an enclosing function.

Note that enforce is throwing an *Exception*, not an *Error*, it does not mean the program is in an invalid state. Throwing an Error, the compiler is allowed to not clean up after itself.

Imagine if throwing an Exception disabled RAII in any enclosing function scopes because it considered that an invalid state.

> Conceptually, bar's destructor is called on that closing brace but we decided to abort mission earlier. Your calling destroy(bar) may or may not be wrong in case of 'err'. Only you know at that point.

If that is the case, a hand written destructor should turn off automatic destruction of members. Either the compiler handles member destruction or it doesn't.

-Steve
5 days ago
On 10/16/20 9:05 AM, Steven Schveighoffer wrote:

> The destruction of members is outside the destructor's purview. It can't
> turn the destruction off, so it should logically be considered part of
> an enclosing function.

Thank you. Makes sense.

Ali

4 days ago
On Friday, 16 October 2020 at 16:00:07 UTC, Steven Schveighoffer wrote:
> On 10/16/20 9:12 AM, tchaloupka wrote:
>
>> So when the exception is thrown within Foo destructor (and it's bad on it's own but can easily happen as destructors aren't nothrow @nogc by default).
>> 
>> Is this behavior expected?
>
> I would say it's a bug. The compiler is going to call the member destructor even if the hand-written destructor does it too. If the compiler wants to take responsibility for cleaning up members, it should take full responsibility. In fact, there is no way to instruct the compiler "I'm handling the destruction of this member", so I don't see why it should matter if you exit the function via exception it should be any different.
>
> -Steve

Thx, added https://issues.dlang.org/show_bug.cgi?id=21322