On Wednesday, 4 March 2015 at 19:36:20 UTC, Ali Çehreli wrote:
> On 03/04/2015 07:43 AM, Steven Schveighoffer wrote:
>> On 3/4/15 8:43 AM, Shachar Shemesh wrote:
>>
>>> I'd expect A's destructor to run, which does not seem to be the case.
>>
>>
>> I believe destructors are not run when you throw inside a constructor.
>> So plan to deallocate if the ctor throws:
>>
>> a = A(var + 1);
>> scope(failure) destroy(a);
>>
>> -Steve
>
> I don't want that. :)
>
> Even C++ gets this right.
>
> Ali

Nop, I do think C++ destroy uninitialized fields.

On 05/03/15 04:09, deadalnix wrote:
> On Wednesday, 4 March 2015 at 19:36:20 UTC, Ali Çehreli wrote:
>> Even C++ gets this right.
>>
>> Ali
>
> Nop, I do think C++ destroy uninitialized fields.

I'm sorry, but you are wrong.

In C++, a destructor is run for "fully constructed" objects. Fully constructed is defined to be an object whose constructor has finished successfully (i.e. - without throwing).

Construction order of an object is:
1. Parent(s), if any, in no particular order (unless employing virtual inheritance, in which case a virtual grandparent is constructed before its descendants)
2. Members, in the order in which they were defined in the class (not the order in which they are listed in the constructor. gcc gives a warning if the two are not the same)
3. The actual object

This means that all members are parents are fully constructed by the time the constructor body starts to run, which means that if it throws, the class itself is the only one for which a destructor isn't going to get called.

This also has the less desirable effect that if you want to call a member's non-default constructor, you need to have all arguments ready before the class' constructor starts to run.

Personally, I think the first is well worth the occasional case where the second becomes a liability.

Shachar

On Thursday, 5 March 2015 at 01:13:53 UTC, weaselcat wrote:
>
> Would be nice if votes on bugzilla mattered, like putting highly voted opened bugs on the front page of dlang.org.

They sort of matter. Some people would choose to fix bugs based off of votes. The tricky part is that the issues with the most votes tend to be the ones that are very difficult to fix. :P

On 3/4/15 2:31 PM, Shachar Shemesh wrote:
> On 04/03/15 17:43, Steven Schveighoffer wrote:
>> On 3/4/15 8:43 AM, Shachar Shemesh wrote:
>>
>>> I'd expect A's destructor to run, which does not seem to be the case.
>>
>>
>> I believe destructors are not run when you throw inside a constructor.
>> So plan to deallocate if the ctor throws:
>>
>> a = A(var + 1);
>> scope(failure) destroy(a);
>>
>> -Steve
>
> That's pretty horrible. So now I need to keep track of the
> implementation of my members to figure out whether it is okay to skip
> the destructor or not.

Why does it matter? If the member is constructed, insert a scope(failure) destroy, otherwise don't. Or don't throw in the ctor and mark it nothrow, then you don't have to worry about it at all.

> All of this from a language that claims to support RAII, which was
> invented precisely so the programmer does not have to keep track of
> those things.

It would be nice if there is a way. D, unlike C++, does not separate construction of members from the code. This means the situation is not as easy to solve.

But an interesting question -- when constructing a struct or class, isn't every member initialized with it's init value anyway? Why would calling dtor on that be bad?

-Steve

On 07/03/15 07:09, Steven Schveighoffer wrote:

>> That's pretty horrible. So now I need to keep track of the
>> implementation of my members to figure out whether it is okay to skip
>> the destructor or not.
>
> Why does it matter? If the member is constructed, insert a
> scope(failure) destroy, otherwise don't.
The whole point of RAII is to prevent errors that happen when the programmer isn't as vigilant as she should be. RAII reduces the amount of vigilance required, thus eliminating errors before they happen.

What you are offering here is the precise opposite of that. Not only do I need to remember to add a scope after each initialization, I need to figure out whether my constructor throws, and whether any constructor I'm calling throws. That's an unrealistic level of work just so RAII can save me work.

> Or don't throw in the ctor and
> mark it nothrow, then you don't have to worry about it at all.
No. I also don't have to worry about using structs that did not mark their constructors nothrow, or, pretty much, anything 3rd part at all.

>
>> All of this from a language that claims to support RAII, which was
>> invented precisely so the programmer does not have to keep track of
>> those things.
>
> It would be nice if there is a way. D, unlike C++, does not separate
> construction of members from the code. This means the situation is not
> as easy to solve.
>
> But an interesting question -- when constructing a struct or class,
> isn't every member initialized with it's init value anyway? Why would
> calling dtor on that be bad?
Now, that I can live with. Define the language that the member contains the init value until replaced by the first assignment in the constructor. Also state that if the compiler feels confident in figuring out that the member will always be initialized by the constructor, it can leave out both init initialization and destruction until after actual initialization.

I think, as far as language standard goes, that is reasonable. It is something I can work with. The only performance penalty compared to current situation is in the exception path, which is slower anyways.

Shachar

Why not handle the scope(failure) cases in the constructor itself?

struct SomeStruct {
    void* ptr;
    void* ptr2;

    this() {
        ptr = malloc(...);
        scope(failure) free(ptr);

        /* Whatever follows, possibly throwing. */

        ptr2 = malloc(...);
        scope(failure) free(ptr2);
    }

    ~this() {
        free(ptr);
        free(ptr2);
    }
}

Now if the constructor throws, the resource which needs to be dealt with, whatever it is, is handled even though the destructor is never invoked. Additionally, you only execute the scope(failure) lines you reach before an exception is thrown, whereas the destructor will try to clean up all of the resources. Now the user of SomeStruct can use it the RAII way, and never have to worry about handling cleanup for it.

Then you can use the some simple rules and apply them consistently.

1. Declare constructors as nothrow where you can.
2. Handle failure cases when you can't declare constructors as nothrow.

On 07/03/15 23:44, w0rp wrote:
> Why not handle the scope(failure) cases in the constructor itself?
>
Because this breaks encapsulation.

struct SomeStruct {
	SomeOtherStruct a;
	MoreStruct b;

	this(int c) {
		this.a = SomeOtherStruct(c);
		this.b = MoreStruct(c+1);
	}
}

This code, as written, reasonable though it may seem, does not work with D's current implementation. If MoreStruct's constructor throws, a will be left with dangling resources. Or not. It really depends on how SomeOtherStruct is implemented.

Which is exactly my point. For things to be kept sane, each struct must be able to completely control its own resources. Once we accept that, however, a simple look at your example shows there is a much cleaner way of doing it:

> struct SomeStruct {
>      SmartPtr ptr;
>      SmartPtr ptr2;
>
>      this() {
>          ptr = malloc(...);
>          // No longer necessary: scope(failure) free(ptr);
>
>          /* Whatever follows, possibly throwing. */
>
>          ptr2 = malloc(...);
>          // No longer necessary: scope(failure) free(ptr2);
>      }
>
>      // No longer necessary: there is nothing left for us to destruct: ~this()
> }

This also avoids the bugs your implementation has, where you failed to override this(this) and opAssign, and thus you both are leaking some memory AND double freeing another. If SmartPtr is implemented correctly, the above, as is, is bug free.

Which is precisely why RAII are so great. They reduce the amount of code you need to write in order to be bug-free. You contain the tough corner cases in the RAII implementation, leaving everyone else to just write their code in peace.

Which is why I think problems such as this are a real setback.

Shachar

On Sunday, 8 March 2015 at 07:02:48 UTC, Shachar Shemesh wrote:
> On 07/03/15 23:44, w0rp wrote:
>> Why not handle the scope(failure) cases in the constructor itself?
>>
> Because this breaks encapsulation.
>
> struct SomeStruct {
> 	SomeOtherStruct a;
> 	MoreStruct b;
>
> 	this(int c) {
> 		this.a = SomeOtherStruct(c);
> 		this.b = MoreStruct(c+1);
> 	}
> }
>
> This code, as written, reasonable though it may seem, does not work with D's current implementation. If MoreStruct's constructor throws, a will be left with dangling resources. Or not. It really depends on how SomeOtherStruct is implemented.
>
> Which is exactly my point. For things to be kept sane, each struct must be able to completely control its own resources. Once we accept that, however, a simple look at your example shows there is a much cleaner way of doing it:
>
>> struct SomeStruct {
>>     SmartPtr ptr;
>>     SmartPtr ptr2;
>>
>>     this() {
>>         ptr = malloc(...);
>>         // No longer necessary: scope(failure) free(ptr);
>>
>>         /* Whatever follows, possibly throwing. */
>>
>>         ptr2 = malloc(...);
>>         // No longer necessary: scope(failure) free(ptr2);
>>     }
>>
>>     // No longer necessary: there is nothing left for us to destruct: ~this()
>> }
>
> This also avoids the bugs your implementation has, where you failed to override this(this) and opAssign, and thus you both are leaking some memory AND double freeing another. If SmartPtr is implemented correctly, the above, as is, is bug free.
>
> Which is precisely why RAII are so great. They reduce the amount of code you need to write in order to be bug-free. You contain the tough corner cases in the RAII implementation, leaving everyone else to just write their code in peace.
>
> Which is why I think problems such as this are a real setback.
>
> Shachar

I only wrote enough to show the really important part. You probably want a resource to not be copyable at all, only moveable, so I'd probably write @disable this(this) and probably also @disable this(). As for a struct with a throwing desctructor creating other structs with throwing destructors, my two rules apply exactly as before. If the struct's constructor throws, release the allocated resources manually with scope(failure) cases. Sure encapsulation will be broken, but at the moment you can do no better.

The only way we can improve on this is to have the destructors of each field called, but not the destructor itself, exactly as C++ does it. One complication, as others have mentioned, is that construction of each field cannot be stated explictly away from the actual constructor, so that might cause some issues.

I'll note that following the rules with non-copyable objects, you won't ever double-free any resources. If a constructor of some object in the constructor throws, its destructor won't be called ever, as you won't hit the following explicit scope(failure) case. You won't have a dangling resource, because the previous objects with scope(failure) cases will be destroyed, but again not double-freed because the struct's destructor won't be run.

So if you want something to work as of right now, do that.

On 08/03/15 15:56, w0rp wrote:

> So if you want something to work as of right now, do that.

I realize that. What I'm saying is that this is unacceptable. This violates D's own standard.

From http://dlang.org/exception-safe.html:
> The RAII (Resource Acquisition Is Initialization) idiom and the try-finally statement form the backbone of the traditional approaches to writing exception safe programming.

Except RAII doesn't work. I won't repeat the reasons, but this behavior and RAII simply cannot exist in the same language.

Shachar