Have you ever done:

if(something) {
   import std.conv;
   throw new Exception("some error " ~ to!string(some_value));
}

Don't you hate it?

* having to import std.conv to see data from your exception is a pain
* it allocates eagerly and thus isn't suitable for a lot of places
* inspecting the data can be a pain as the string is unstructured

This assumes the data is even bothered to be added. Anyone who has gotten a RangeError in D knows important information is often just dropped!

A good solution is to make a new exception subclass for each error type, storing details as data members. However, that's a bit of a pain in D because of all the work you have to do to make a complete subclass:

* you need to forward the constructors
* you need to reimplement the toString to print out data members
* you still have a string member there that is encouraged to be used


Finally, with the discussion of @nogc exceptions, it would be nice to simplify the constructor and get file/line info separated from it at the same time too. Can we tackle all these problems at once?

I propose we do some library changes, one minor compiler change, and a culture shift toward better exceptions. Here's my proof of concept:

http://arsdnet.net/dcode/exception.d


Read that code and the comments to see what I'm going for and what it looks like. Here's the summary:

* The Exception constructor is simplified: no required arguments, everything else is done with members. (The Throwable next = null argument might come back, but it can also be set after construction anyway so I'd prefer not to worry about it in most places either.)

* The string is de-emphasized. It is still there so we don't break all the code that uses it now, but it is no longer the main thing and we really would prefer that it isn't used at all. Exception messages are instead done with the class name and data members. The string may be gotten by the class though in cases like converting errno to a message.

* File and line is set at the throw point instead of the construction point.

* A mixin template uses reflection to print data to the user instead of constructing a string.

* This is allocation-free, except for the exception object itself. User code would NOT have to import std.conv, it just stores the data in the object.



Changes:

TO THE COMPILER:
  * make the call to d_throw add file and line number information. This is completely backward compatible and allows the runtime to store it in the object if it wants to. Then my "fly" method becomes unnecessary.

TO THE LIBRARY:
  * Throwable's toString implementation changes a bit to call a couple more virtual functions to print details. Very similar to the current code, as you can see in my file there, just some overridable hooks.

  * A PrintMembers and perhaps other helper mixins are added to the library. Doesn't matter where they go, druntime might do a simpler one for its own needs and then Phobos offers a full featured one for user code. Or whatever. It'd need to be a bit more complex than my proof of concept to cover more data (at least let it try calling toString on types too) but this is already pretty useful as is.


TO USER CODE:
  * We start using subclasses with data members instead of string arguments, migrating to it incrementally and encouraging people to write exceptions this way going forward.

  * Note that this should be completely backward compatible, it won't break old code, just encourage a new better way.



What do you all think? I encourage you to grab my example there and play with it a bit to see how you like it too and see if you can think of any improvements.

D's exceptions kinda suck right now but they don't have to!

I didn't give much attention to the details. but I like the spirit very much.

LMB


On Wed, May 13, 2015 at 12:08 PM, Adam D. Ruppe via Digitalmars-d < digitalmars-d@puremagic.com> wrote:

> Have you ever done:
>
> if(something) {
>    import std.conv;
>    throw new Exception("some error " ~ to!string(some_value));
> }
>
> Don't you hate it?
>
> * having to import std.conv to see data from your exception is a pain
> * it allocates eagerly and thus isn't suitable for a lot of places
> * inspecting the data can be a pain as the string is unstructured
>
> This assumes the data is even bothered to be added. Anyone who has gotten a RangeError in D knows important information is often just dropped!
>
> A good solution is to make a new exception subclass for each error type, storing details as data members. However, that's a bit of a pain in D because of all the work you have to do to make a complete subclass:
>
> * you need to forward the constructors
> * you need to reimplement the toString to print out data members
> * you still have a string member there that is encouraged to be used
>
>
> Finally, with the discussion of @nogc exceptions, it would be nice to simplify the constructor and get file/line info separated from it at the same time too. Can we tackle all these problems at once?
>
> I propose we do some library changes, one minor compiler change, and a culture shift toward better exceptions. Here's my proof of concept:
>
> http://arsdnet.net/dcode/exception.d
>
>
> Read that code and the comments to see what I'm going for and what it looks like. Here's the summary:
>
> * The Exception constructor is simplified: no required arguments, everything else is done with members. (The Throwable next = null argument might come back, but it can also be set after construction anyway so I'd prefer not to worry about it in most places either.)
>
> * The string is de-emphasized. It is still there so we don't break all the code that uses it now, but it is no longer the main thing and we really would prefer that it isn't used at all. Exception messages are instead done with the class name and data members. The string may be gotten by the class though in cases like converting errno to a message.
>
> * File and line is set at the throw point instead of the construction point.
>
> * A mixin template uses reflection to print data to the user instead of constructing a string.
>
> * This is allocation-free, except for the exception object itself. User code would NOT have to import std.conv, it just stores the data in the object.
>
>
>
> Changes:
>
> TO THE COMPILER:
>   * make the call to d_throw add file and line number information. This is
> completely backward compatible and allows the runtime to store it in the
> object if it wants to. Then my "fly" method becomes unnecessary.
>
> TO THE LIBRARY:
>   * Throwable's toString implementation changes a bit to call a couple
> more virtual functions to print details. Very similar to the current code,
> as you can see in my file there, just some overridable hooks.
>
>   * A PrintMembers and perhaps other helper mixins are added to the
> library. Doesn't matter where they go, druntime might do a simpler one for
> its own needs and then Phobos offers a full featured one for user code. Or
> whatever. It'd need to be a bit more complex than my proof of concept to
> cover more data (at least let it try calling toString on types too) but
> this is already pretty useful as is.
>
>
> TO USER CODE:
>   * We start using subclasses with data members instead of string
> arguments, migrating to it incrementally and encouraging people to write
> exceptions this way going forward.
>
>   * Note that this should be completely backward compatible, it won't
> break old code, just encourage a new better way.
>
>
>
> What do you all think? I encourage you to grab my example there and play with it a bit to see how you like it too and see if you can think of any improvements.
>
> D's exceptions kinda suck right now but they don't have to!
>

On 2015-05-13 17:08, Adam D. Ruppe wrote:
> Have you ever done:
>
> if(something) {
>     import std.conv;
>     throw new Exception("some error " ~ to!string(some_value));
> }
>
> Don't you hate it?
>
> * having to import std.conv to see data from your exception is a pain
> * it allocates eagerly and thus isn't suitable for a lot of places
> * inspecting the data can be a pain as the string is unstructured
>
> This assumes the data is even bothered to be added. Anyone who has
> gotten a RangeError in D knows important information is often just dropped!
>
> A good solution is to make a new exception subclass for each error type,
> storing details as data members. However, that's a bit of a pain in D
> because of all the work you have to do to make a complete subclass:

Yeah, I really hate that people are using plain Exception instead of creating a subclass. I'm trying to point this out in pull requests and similar but it's hard to get people to listen.

One thing that is _not_ making things better is "enforce" which, if I recall correctly, throws Exception by default.

I think this is related: http://wiki.dlang.org/DIP33

-- 
/Jacob Carlborg

On Wednesday, 13 May 2015 at 19:24:09 UTC, Jacob Carlborg wrote:
> Yeah, I really hate that people are using plain Exception instead of creating a subclass. I'm trying to point this out in pull requests and similar but it's hard to get people to listen.
>

With how range focused/@nogc a lot of phobos code has become exceptions have almost become useless, and we're back to throwing errors anyways.
Bad situation.

On 5/13/15 3:24 PM, Jacob Carlborg wrote:
> On 2015-05-13 17:08, Adam D. Ruppe wrote:
>> Have you ever done:
>>
>> if(something) {
>>     import std.conv;
>>     throw new Exception("some error " ~ to!string(some_value));
>> }
>>
>> Don't you hate it?
>>
>> * having to import std.conv to see data from your exception is a pain
>> * it allocates eagerly and thus isn't suitable for a lot of places
>> * inspecting the data can be a pain as the string is unstructured
>>
>> This assumes the data is even bothered to be added. Anyone who has
>> gotten a RangeError in D knows important information is often just
>> dropped!
>>
>> A good solution is to make a new exception subclass for each error type,
>> storing details as data members. However, that's a bit of a pain in D
>> because of all the work you have to do to make a complete subclass:
>
> Yeah, I really hate that people are using plain Exception instead of
> creating a subclass. I'm trying to point this out in pull requests and
> similar but it's hard to get people to listen.
>
> One thing that is _not_ making things better is "enforce" which, if I
> recall correctly, throws Exception by default.

enforce is one of the most needless pieces of phobos:

enforce(cond, message);
vs.
if(!cond) throw new Exception(message);

And the second doesn't mess up inlining.

I think enforce could be boiler-plated better. The only verbose part of the if version is the throwing and newing.

template throe(Etype = Exception)
{
   void throe(Args...)(Args args, string file = __FILE__, size_t line = __LINE__)
   {
   	throw new Etype(args, file, line);
   }
}

if(!cond) throe(message);

Wait, you're in an io package, and you want to always throw IO exceptions?

alias except = throe!IOException;

if(!cond) except(args, to, ioexception);

Sure, it doesn't return the thing that caused the exception if nothing happens. Grepping phobos, this feature is used with enforce about 1% of the time. In fact, I didn't even know it had that feature until looking it up in the docs just now.

-Steve

On Wednesday, 13 May 2015 at 19:24:09 UTC, Jacob Carlborg wrote:
> One thing that is _not_ making things better is "enforce" which, if I recall correctly, throws Exception by default.

Aye. enforce is one of those things that looks cool but I don't think should actually be used.

It could perhaps be fixed...

alias enforce = enforceBase!FileException;

FILE* fp = enforce!fopen("test.txt", "rt");


The local alias tells which kind of exception is relevant in this context. Then the local enforce collects the arguments to the function and throws a new subclass of the base specific to this function call.

There's arguably value in that, but still, I'm not sure it is better than just doing it yourself.

Refresh this:
http://arsdnet.net/dcode/exception.d


It now has my enforce 2.0 proof of concept draft.

Usage:
        alias enforce = enforceBase!MyExceptionBase;
        import core.stdc.stdio;
        enforce!fopen("nofile.txt".ptr, "rb".ptr);

Message:

MyExceptionBase@exception.d(38): fopen call failed
        filename = nofile.txt
        mode = rb
----------------
stack trace here


That's actually kinda cool, automatically populating the exception with the arguments to the call. Perhaps of some value (and I don't think it will break inline since it uses a function alias argument instead of lazy params!)


> I think this is related: http://wiki.dlang.org/DIP33

Yeah, we could use a decent hierarchy too. Though the examples in there still use string concatenation to form the message, which is the big thing I want to get away from.

On 2015-05-14 03:31, Adam D. Ruppe wrote:

> Yeah, we could use a decent hierarchy too. Though the examples in there
> still use string concatenation to form the message, which is the big
> thing I want to get away from.

It was a while since I looked at that DIP, but I'm mostly interested in the hierarchy.

-- 
/Jacob Carlborg

On 2015-05-14 00:55, Steven Schveighoffer wrote:

> enforce is one of the most needless pieces of phobos:
>
> enforce(cond, message);
> vs.
> if(!cond) throw new Exception(message);
>
> And the second doesn't mess up inlining.
>
> I think enforce could be boiler-plated better. The only verbose part of
> the if version is the throwing and newing.
>
> template throe(Etype = Exception)
> {
>     void throe(Args...)(Args args, string file = __FILE__, size_t line =
> __LINE__)
>     {
>         throw new Etype(args, file, line);
>     }
> }
>
> if(!cond) throe(message);

Now you're back to the same problem as "enforce" has. That it throws Exception by default. It shouldn't have a default value for the exception type.

BTW, it could be called "raise" as it's called in some other languages.

> Wait, you're in an io package, and you want to always throw IO exceptions?
>
> alias except = throe!IOException;
>
> if(!cond) except(args, to, ioexception);

That is a bit better but I still think that IOException is too generic. Streaming something over the network and trying to open a file is quite different.

> Sure, it doesn't return the thing that caused the exception if nothing
> happens. Grepping phobos, this feature is used with enforce about 1% of
> the time. In fact, I didn't even know it had that feature until looking
> it up in the docs just now.
>
> -Steve


-- 
/Jacob Carlborg

On Thursday, 14 May 2015 at 01:31:22 UTC, Adam D. Ruppe wrote:
> The local alias tells which kind of exception is relevant in this context. Then the local enforce collects the arguments to the function and throws a new subclass of the base specific to this function call.

Using alias like this makes code hard to read. Error types should be humanly deducible at the failure site.

You'd be better off having non-ignorable result types (e.g. tagged union/variant/algebraic) and a typed way to turn those into exceptions.

On Wednesday, 13 May 2015 at 15:08:58 UTC, Adam D. Ruppe wrote:
> * File and line is set at the throw point instead of the construction point.

Maybe also replace file name with ModuleInfo similar to how assert works?