May 25, 2017
On 5/25/17 6:24 AM, Walter Bright wrote:
> On 5/23/2017 3:40 PM, Martin Nowak wrote:
>> Why does it have to be refcounted? Seems like there is only ever one reference to the current exception (the catch variable).
> 
> Rethrowing the catch variable makes for 2 references.

Why doesn't the rethrow count as a move? There is no way it can be reused in the scope that rethrows. -- Andrei
May 25, 2017
On Friday, 19 May 2017 at 15:45:28 UTC, Mike Parker wrote:
> DIP 1008 is titled "Exceptions and @nogc".
>
> https://github.com/dlang/DIPs/blob/master/DIPs/DIP1008.md
>
> All review-related feedback on and discussion of the DIP should occur in this thread. The review period will end at 11:59 PM ET on June 2 (3:59 AM GMT June 3), or when I make a post declaring it complete.
>
> At the end of Round 1, if further review is deemed necessary, the DIP will be scheduled for another round. Otherwise, it will be queued for the formal review and evaluation by the language authors.
>
> Extensive discussion of this DIP has already taken place in two threads, both linked from the document. You may find it beneficial to skim through those threads before posting any feedback here.
>
> Thanks in advance to all who participate.
>
> Destroy!

As many others, I dislike special-casing "throw new" and enough has been said about that.

I also don't like adding hooks to druntime, or that by doing so the allocation strategy is baked in (but I guess only when using the `new` operator so there's that).

I think maybe the problem isn't with `throw` but with `catch`. What if instead we make it so that:

catch(scope T ex) { /*...*/; }

Means:

catch(scope T ex) { /*...*/; ex.__dtor; }


That way whoever is throwing the exception allocates however he or she sees fit and the memory is handled by the class's destructor. Sort of proof of concept using malloc (if I did this for real I'd use std.allocator):

class MallocException: Exception {

    char* buffer;
    size_t length;

    // has to be public or `emplace` won't compile
    this(string msg) @nogc {
        import core.stdc.stdlib: malloc;
        length = msg.length;
        buffer = cast(char*)malloc(length);
        buffer[0 .. length] = msg[];
        super(cast(string)buffer[0 .. length]);
    }

    ~this() @nogc {
        import core.stdc.stdlib: free;
        free(buffer);
        free(cast(void*)this);
    }

    static MallocException create(string msg) @nogc {
        import core.stdc.stdlib: malloc;
        import std.conv: emplace;
        enum size = __traits(classInstanceSize, MallocException);
        auto buf = malloc(size);
        return emplace!MallocException(buf[0 .. size], msg);
    }
}

void main() @nogc {
    try {
        import core.stdc.stdlib: malloc;
        throw MallocException.create("oops");
    } catch(MallocException ex) {
        ex.__dtor;
    }
}

The code above works today, the annoying thing is having to invoke the destructor manually (and also knowing it's called `__dtor`).

This would however mean delaying making phobos @nogc since each `throw` site would have to be changed.

And if someone never remembers to catch by scope somewhere and is allocating memory themselves, well... use the GC, Luke.

Atila
May 26, 2017
On 5/25/2017 12:30 AM, Andrei Alexandrescu wrote:
> On 5/25/17 6:24 AM, Walter Bright wrote:
>> On 5/23/2017 3:40 PM, Martin Nowak wrote:
>>> Why does it have to be refcounted? Seems like there is only ever one reference to the current exception (the catch variable).
>>
>> Rethrowing the catch variable makes for 2 references.
> 
> Why doesn't the rethrow count as a move? There is no way it can be reused in the scope that rethrows. -- Andrei

It could be - it's just that there's nothing currently in the compiler to support the notion of moves. So the destructor will still wind up getting called on it.

It's a good idea for a future enhancement, though.
May 26, 2017
On 5/25/2017 4:54 PM, Atila Neves wrote:
> I think maybe the problem isn't with `throw` but with `catch`. What if instead we make it so that:
> 
> catch(scope T ex) { /*...*/; }
> 
> Means:
> 
> catch(scope T ex) { /*...*/; ex.__dtor; }

The trouble comes in when one starts copying exception references around. Who then is responsible for destroying it?
May 26, 2017
On 5/26/17 4:49 AM, Walter Bright wrote:
> On 5/25/2017 4:54 PM, Atila Neves wrote:
>> I think maybe the problem isn't with `throw` but with `catch`. What if
>> instead we make it so that:
>>
>> catch(scope T ex) { /*...*/; }
>>
>> Means:
>>
>> catch(scope T ex) { /*...*/; ex.__dtor; }
>
> The trouble comes in when one starts copying exception references
> around. Who then is responsible for destroying it?

This isn't the trouble. The trouble is that `new Exception` is not @nogc, and there isn't a way to fix all existing exception code easily.

But to answer your question, don't mark the exception scope in that case. The compiler should complain if you copy it outside the scope, no?

My $0.02: Either we are going to make `new Exception` be @nogc, or we are going to require people to type something different. IMO, I feel if we can create a library solution, and use dfix or similar tool to allow people to update their code, then we are in a much better place.

I need to review the DIP again, but last time I read it, there were some issues I saw. I will post those in a while.

-Steve
May 26, 2017
On 5/26/2017 4:50 AM, Steven Schveighoffer wrote:
> But to answer your question, don't mark the exception scope in that case. The compiler should complain if you copy it outside the scope, no?

I suspect if you proceed with that line of reasoning, you'll wind up in the same place I did with DIP 1008.
May 26, 2017
On Friday, 26 May 2017 at 08:49:42 UTC, Walter Bright wrote:
> On 5/25/2017 4:54 PM, Atila Neves wrote:
>> I think maybe the problem isn't with `throw` but with `catch`. What if instead we make it so that:
>> 
>> catch(scope T ex) { /*...*/; }
>> 
>> Means:
>> 
>> catch(scope T ex) { /*...*/; ex.__dtor; }
>
> The trouble comes in when one starts copying exception references around. Who then is responsible for destroying it?

Since it's `scope`, where would it be copied to? This is assuming dip1000, of course.

Atila
May 26, 2017
On Friday, 26 May 2017 at 11:50:40 UTC, Steven Schveighoffer wrote:
> On 5/26/17 4:49 AM, Walter Bright wrote:
>> On 5/25/2017 4:54 PM, Atila Neves wrote:
>>> I think maybe the problem isn't with `throw` but with `catch`. What if
>>> instead we make it so that:
>>>
>>> catch(scope T ex) { /*...*/; }
>>>
>>> Means:
>>>
>>> catch(scope T ex) { /*...*/; ex.__dtor; }
>>
>> The trouble comes in when one starts copying exception references
>> around. Who then is responsible for destroying it?
>
> This isn't the trouble. The trouble is that `new Exception` is not @nogc, and there isn't a way to fix all existing exception code easily.

True, but a hypothetical `NoGcException.create` _is_ `@nogc` (same as my MallocException), and is there really any difference between reading/writing `new Foo` and `Foo.create`?

Also, there's `enforce`, which is where I suspect a lot of throwing happens anyway. It could use DbI to figure out from the exception type what to do.

> My $0.02: Either we are going to make `new Exception` be @nogc, or we are going to require people to type something different.

Or be able to customise `new T`, which feels less hacky than it magically meaning something different if the type is a Throwable. I know that there were class allocators in D1 and that they're depecreated but I wasn't around back then to know why.

Atila

May 26, 2017
On Friday, 26 May 2017 at 16:36:16 UTC, Walter Bright wrote:
> On 5/26/2017 4:50 AM, Steven Schveighoffer wrote:
>> But to answer your question, don't mark the exception scope in that case. The compiler should complain if you copy it outside the scope, no?
>
> I suspect if you proceed with that line of reasoning, you'll wind up in the same place I did with DIP 1008.

Could you explain the line of reasoning that led you do dip1008? Thanks,

Atila
May 26, 2017
On 5/26/17 2:58 PM, Atila Neves wrote:
> On Friday, 26 May 2017 at 11:50:40 UTC, Steven Schveighoffer wrote:
>> On 5/26/17 4:49 AM, Walter Bright wrote:
>>> On 5/25/2017 4:54 PM, Atila Neves wrote:
>>>> I think maybe the problem isn't with `throw` but with `catch`. What if
>>>> instead we make it so that:
>>>>
>>>> catch(scope T ex) { /*...*/; }
>>>>
>>>> Means:
>>>>
>>>> catch(scope T ex) { /*...*/; ex.__dtor; }
>>>
>>> The trouble comes in when one starts copying exception references
>>> around. Who then is responsible for destroying it?
>>
>> This isn't the trouble. The trouble is that `new Exception` is not
>> @nogc, and there isn't a way to fix all existing exception code easily.
>
> True, but a hypothetical `NoGcException.create` _is_ `@nogc` (same as my
> MallocException), and is there really any difference between
> reading/writing `new Foo` and `Foo.create`?

There isn't, but Foo.create doesn't exist in current code. Someone has to go through it all and update.

Note that the implication in your strawman is that you need a special exception to be nogc. I'd rather leave the (de)allocation of the exception up to the thrower/catcher, and have the exception not care about its own location.

> Also, there's `enforce`, which is where I suspect a lot of throwing
> happens anyway. It could use DbI to figure out from the exception type
> what to do.

Good point, we could change enforce, and that would fix a lot of the usage of exceptions.

>
>> My $0.02: Either we are going to make `new Exception` be @nogc, or we
>> are going to require people to type something different.
>
> Or be able to customise `new T`, which feels less hacky than it
> magically meaning something different if the type is a Throwable. I know
> that there were class allocators in D1 and that they're depecreated but
> I wasn't around back then to know why.

Deprecated, but still there.

However, it's the auto-destruction that isn't there. There isn't a way to tell the compiler to destroy on the catch scope automatically. Currently, when you override class allocation, you need to explicitly delete.

-Steve