Dear community, are you ready for this?

https://issues.dlang.org/show_bug.cgi?id=2834
https://github.com/D-Programming-Language/druntime/pull/864

We must do it, and the way I see it the earlier the better. Shall we do it in 2.067?

This is a significant change of behavior. Should we provide a temporary flag or attribute to disable it?


Thanks,

Andrei

On 8/28/2014 9:21 PM, Andrei Alexandrescu wrote:
> Dear community, are you ready for this?
>
> https://issues.dlang.org/show_bug.cgi?id=2834
> https://github.com/D-Programming-Language/druntime/pull/864
>
> We must do it, and the way I see it the earlier the better. Shall we do
> it in 2.067?
>
> This is a significant change of behavior. Should we provide a temporary
> flag or attribute to disable it?
>
>
> Thanks,
>
> Andrei

There should be no breaking behavior, provided the code was previously calling destroy, and not simply manually calling the destructor directly for a heap allocated struct, as I've had to make sure that it's not called more than once, otherwise the existing unittests wouldn't pass.

The only potentially breaking change introduced in this, which was previously present in one of the struct destructor tests, is that allocating in the heap allocated struct's destructor isn't allowed.

On Thu, Aug 28, 2014 at 07:21:04PM -0700, Andrei Alexandrescu via Digitalmars-d wrote:
> Dear community, are you ready for this?
> 
> https://issues.dlang.org/show_bug.cgi?id=2834 https://github.com/D-Programming-Language/druntime/pull/864
> 
> We must do it, and the way I see it the earlier the better. Shall we do it in 2.067?

Let's do it. Issue 2834 was filed in 2009. It's almost winning the longest-open-bug award. It's about time we did something.


> This is a significant change of behavior. Should we provide a temporary flag or attribute to disable it?
[...]

Maybe a more relevant question might be, is there any existing code that *isn't* broken by structs not being destructed? (D-structed, har har.)


T

-- 
If I were two-faced, would I be wearing this one? -- Abraham Lincoln

On Friday, 29 August 2014 at 02:21:07 UTC, Andrei Alexandrescu
wrote:
> Dear community, are you ready for this?
>
> https://issues.dlang.org/show_bug.cgi?id=2834
> https://github.com/D-Programming-Language/druntime/pull/864
>
> We must do it, and the way I see it the earlier the better. Shall we do it in 2.067?
>
> This is a significant change of behavior. Should we provide a temporary flag or attribute to disable it?
>
>
> Thanks,
>
> Andrei

I have no idea what the decision is.

On Friday, 29 August 2014 at 02:21:07 UTC, Andrei Alexandrescu wrote:
> Dear community, are you ready for this?
>
> https://issues.dlang.org/show_bug.cgi?id=2834
> https://github.com/D-Programming-Language/druntime/pull/864
>
> We must do it, and the way I see it the earlier the better. Shall we do it in 2.067?
>
> This is a significant change of behavior. Should we provide a temporary flag or attribute to disable it?

It will _fix_ a lot of code, I expect, whereas it will probably break very little. The main thing that it would break would be if there's any GC heap allocation in a struct's destructor, but that will only matter if the struct is on the GC heap, and I think that it's something that we're pretty much stuck with. Certainly, that's not much of a loss in comparison to all of the bugs created by stuff like an array of std.stdio.Files. Maybe there's a problem if someone was manually destroying stuff? But if so, I don't see how we can avoid breaking their code. And they'll probably be much happier with this change just so long as they're aware of it, and hopefull the changelog is enough for that.

So, I'd say to put it in 2.067. I don't think that waiting buys us anything.

- Jonathan M Davis

On 8/28/2014 7:21 PM, Andrei Alexandrescu via Digitalmars-d wrote:
> Dear community, are you ready for this?
>
> https://issues.dlang.org/show_bug.cgi?id=2834
> https://github.com/D-Programming-Language/druntime/pull/864
>
> We must do it, and the way I see it the earlier the better. Shall we do
> it in 2.067?
>
> This is a significant change of behavior. Should we provide a temporary
> flag or attribute to disable it?
>
>
> Thanks,
>
> Andrei

I didn't study the changes, except to note that the number of tests seems rather considering the nature of what's changing.

On 8/28/2014 7:54 PM, Brad Roberts via Digitalmars-d wrote:
> I didn't study the changes, except to note that the number of tests
> seems rather considering the nature of what's changing.

Er: rather LOW considering...

On Friday, 29 August 2014 at 02:21:07 UTC, Andrei Alexandrescu wrote:
> Dear community, are you ready for this?
>
> https://issues.dlang.org/show_bug.cgi?id=2834
> https://github.com/D-Programming-Language/druntime/pull/864
>
> We must do it, and the way I see it the earlier the better. Shall we do it in 2.067?
>
> This is a significant change of behavior. Should we provide a temporary flag or attribute to disable it?
>
>
> Thanks,
>
> Andrei

I think providing rollback transition flag is simply a matter of politeness even if in this specific case it makes no sense to rely on existing broken behavior.

On Friday, 29 August 2014 at 02:59:07 UTC, Dicebot wrote:
> On Friday, 29 August 2014 at 02:21:07 UTC, Andrei Alexandrescu wrote:
>> Dear community, are you ready for this?
>>
>> https://issues.dlang.org/show_bug.cgi?id=2834
>> https://github.com/D-Programming-Language/druntime/pull/864
>>
>> We must do it, and the way I see it the earlier the better. Shall we do it in 2.067?
>>
>> This is a significant change of behavior. Should we provide a temporary flag or attribute to disable it?
>>
>>
>> Thanks,
>>
>> Andrei
>
> I think providing rollback transition flag is simply a matter of politeness even if in this specific case it makes no sense to rely on existing broken behavior.

Since this is in the library, not the compiler, I'm not sure how you'd do that (have the compiler specific a version identifier for it?), but considering how broken the behavior was before and that we don't normally provide flags to revert changes in behavior, I'd argue that anyone who has problems with it should just use the previous version of the compiler until they're ready to update their code. It's not like we want to support the old behavior for any real period of time anyway. It's just plain broken.

- Jonathan M Davis

On 8/28/14, 8:07 PM, Jonathan M Davis wrote:
> Since this is in the library, not the compiler, I'm not sure how you'd
> do that (have the compiler specific a version identifier for it?), but
> considering how broken the behavior was before and that we don't
> normally provide flags to revert changes in behavior, I'd argue that
> anyone who has problems with it should just use the previous version of
> the compiler until they're ready to update their code.

I'd say the impact is too large to leave things at that. How about a function call callStructDestructorsDuringGC(bool) - yes, that long - that people can place in main() if they're having trouble?

Andrei