View mode: basic / threaded / horizontal-split · Log in · Help
April 02, 2013
Re: DIP33: A standard exception hierarchy
02-Apr-2013 22:26, Jonathan M Davis пишет:
> On Tuesday, April 02, 2013 22:16:30 Dmitry Olshansky wrote:
>> 02-Apr-2013 21:48, Walter Bright пишет:
>>> On 4/2/2013 4:59 AM, Dmitry Olshansky wrote:
>>>> If somebody comes up with a reasonable Visitor pattern for Exceptions
>>>> that is
>>>> flexible and fast then sure let's see it.
>>>
>>> It doesn't really need to be fast. If you need performance out of
>>> Exceptions, you're misusing the idiom.
>>
>> The exceptions are slowish but that hardly justifies adding an extra
>> amount of overhead on top of that. This might as well push people to
>> avoid them even where it makes perfect sense to use exceptions.
>>
>> That being said let's see that beast and then measure, then optimize and
>> then judge it.
>
> D's exceptions are ridiculously slow (
> http://d.puremagic.com/issues/show_bug.cgi?id=9584 ).

Make sure you did the magic setStackTraceHandler(null);
or something to that effect I need to peruse druntime API to pinpoint 
the exact name.

It seems like debug hook dealing with traces is abhorrently slow and is 
called regardless of if you need to print it or not.

Otherwise they are exceptions like any other.

> Granted, in general, you
> shouldn't be relying on exceptions being efficient (they _are_ the error code
> path after all), but we really should do better than we're currently doing,
> and adding extra overhead obviously wouldn't help.
>

> The main area that I find exception speed to be a real problem is in unit
> testing. Solid unit tests will test error conditions, which generally means
> using assertThrown to verify that the correct exception was thrown for bad
> input, but with how slow D's exceptions are, it becomes _very_ expensive to do
> many tests like that, which is very frustrating when you're trying to do
> thorough unit tests.

Indeed exceptions vs error codes shouldn't be orders of magnitude deal.
If it is we need to do better job in implementation quality department.

-- 
Dmitry Olshansky
April 02, 2013
Re: DIP33: A standard exception hierarchy
On 2013-04-02 19:02, Steven Schveighoffer wrote:

> Right, but what I see here is that the language uses one set of criteria
> to determine whether it should catch, but it's difficult to use that
> same criteria in order to process the exception.  It's not easy to
> switch on a class type, in fact it's downright ugly (maybe we need to
> come up with a way to do that in normal code too).

That's kind of breaking the whole point of OO and virtual functions, you 
should not need to know the exact type.


> Yes.  I won't forget to re-throw the exception.  Plus, it seems that you
> are saying "catch this, but if it's also that, then *really* catch it".
> I think the catch is a one-shot deal, and should be the final
> disposition of the exception, you should rarely have to re-throw.
> Re-throwing has it's own problems too, consider this possibility:
>
> catch(ErrnoException ex) if(ex.errno == EBADF || ex.errno == EBADPARAM)
> {
>     // handle these specifically
> }
> catch(Exception ex)
> {
>     // handle all other exceptions
> }
>
> I think you would have to have nested try/catch statements to do that
> without something like this.

I'm just trying to exhaust all exiting language constructs first, before 
adding new ones.

> I mean it's a waste of code space and template bloat.  Not a waste to
> create the exception.

I see.

-- 
/Jacob Carlborg
April 02, 2013
Re: DIP33: A standard exception hierarchy
On Monday, 1 April 2013 at 22:46:49 UTC, Ali Çehreli wrote:
> But I also remember that an AssertError may be thrown by an 
> assert() call, telling us that a programmer put it in there 
> explicitly, meaning that the program cannot continue. If there 
> was any chance of recovery, then the programmer could have 
> thrown an Exception or remedy the situation right then.
>
> Ali

I don't think assert/Error makes any statement on the ability to 
recover. What it usually means is you need to fix this because I 
won't be checking this condition when you throw on that release 
flag.

If you are doing input validation you should be throwing an 
exception.

We can still throw exceptions in production, I don't tend to use 
this, but maybe this would be a time to say "invalid state stop." 
But then how do you distinguish it from "fix your program?"

I've mostly enjoyed having temporary files being cleaned up upon 
some range access error which has no effect on my removing files 
that are no longer valid.
April 02, 2013
Re: DIP33: A standard exception hierarchy
On 04/02/2013 12:01 PM, Jesse Phillips wrote:> On Monday, 1 April 2013 
at 22:46:49 UTC, Ali Çehreli wrote:
>> But I also remember that an AssertError may be thrown by an assert()
>> call, telling us that a programmer put it in there explicitly, meaning
>> that the program cannot continue. If there was any chance of recovery,
>> then the programmer could have thrown an Exception or remedy the
>> situation right then.
>>
>> Ali
>
> I don't think assert/Error makes any statement on the ability to
> recover.

Agreed. Error says that the program state is in an unknown state, while 
Exception says that we could not complete our task.

> What it usually means is you need to fix this because I won't
> be checking this condition when you throw on that release flag.

Aside: That's why the release flag should be used only when one is sure 
that all possible runtime scenarios are tested. I don't understand how 
anybody can be sure of that though. :)

> If you are doing input validation you should be throwing an exception.

Agreed.

> We can still throw exceptions in production, I don't tend to use this,
> but maybe this would be a time to say "invalid state stop." But then how
> do you distinguish it from "fix your program?"

(I am not sure that I understand that comment correctly.)

The benefit of exceptions is that a low level function throws an 
exception if it cannot achieve its task. This is very different from 
incorrect program state. Then, a higher level function (usually all the 
way at the user interaction layer), catches this exception and reports 
"I could not do it because of such and such."

The program state is still good because we have cleaned up everything as 
a result of stack unwinding.

> I've mostly enjoyed having temporary files being cleaned up upon some
> range access error which has no effect on my removing files that are no
> longer valid.

The problem is, the runtime cannot know that it will be doing what you 
really wanted. The incorrect program state may result in deleting the 
wrong file.

Ali
April 02, 2013
Re: DIP33: A standard exception hierarchy
On Monday, 1 April 2013 at 23:03:56 UTC, Jonathan M Davis wrote:
> On Monday, April 01, 2013 13:08:15 Lars T. Kyllingstad wrote:
>> It's time to clean up this mess.
>> 
>> http://wiki.dlang.org/DIP33
>
> Another concern I have is InvalidArgumentError. There _are_ 
> cases where it
> makes sense for invalid arguments to be an error, but there are 
> also plenty of
> cases where it should be an exception (TimeException is 
> frequently used in
> that way), so we may or may not want an 
> InvalidArgumentException, but if you
> do that, you run the risk of making it too easy to confuse the 
> two, thereby
> causing nasty bugs. And most of the cases where 
> InvalidArgumentError makes
> sense could simply be an assertion in an in contract, so I 
> don't know that
> it's really needed or even particularly useful. In general, I 
> think that
> having a variety of Exception types is valuable, because you 
> catch exceptions
> based on their type, but with Errors, you're really not 
> supposed to catch
> them, so having different Error types is of questionable value. 
> That doesn't
> mean that we shouldn't ever do it, but they need a very good 
> reason to exist
> given the relative lack of value that they add.

I definitely don't think we need an IllegalArgumentException.  
IMO, passing illegal arguments is 1) a simple programming error, 
in which case it should be an Error, or 2) something the 
programmer can not avoid, in which case it requires a better 
description of why things went wrong than just "illegal 
argument".  "File not found", for example.

I didn't really consider contracts when I wrote the DIP, and of 
course there will always be the problem of "should this be a 
contract or a normal input check?"  The problem with contracts, 
though, is that they go away in release mode, which is certainly 
not safe if that is your error handling mechanism.


> Also, if you're suggesting that these be _all_ of the exception 
> types in
> Phobos, I don't agree. I think that there's definite value in 
> having specific
> exception types for specific sections of code (e.g. 
> TimeException for time-
> related code or CSVException in std.csv). It's just that they 
> should be put in
> a proper place in the hierarchy so that users of the library 
> can choose to
> catch either the base class or the derived class depending on 
> how specific
> their error handling needs to be and on whatever else their 
> code is doing. We
> _do_ want to move away from simply declaring module-specific 
> exception types,
> but sometimes modules _should_ have specific exception types.

There may of course be, and probably are, a need for more 
exceptions than what I've listed in the DIP.  The idea was to 
make a pattern, a system, to which more exceptions can be added 
if strictly necessary.  I do think, however, that we should try 
to keep the number at a minimum, and that we should NOT create 
new classes for every little detail that can go wrong.


> The focus needs to be on creating a hierarchy that aids in 
> error handling, so
> what exceptions we have should be based on what types of things 
> it makes sense
> to catch in order to handle those errors specifically rather 
> than them being
> treated as a general error, or even a general error of a 
> specific category.
> Having a solid hierarchy is great and very much needed, but I 
> fear that your
> DIP is too focused on getting rid of exception types rather 
> than shifting them
> into their proper place in the exception hierarchy. In some 
> cases, that _does_
> mean getting rid of exception types, but I think that on the 
> whole, it's more
> of a case of creating new base classes for existing exceptions 
> so that we have
> key base classes in the hierarchy. The DIP focuses on those 
> base classes but
> seems to want to get rid of the rest, and I think that that's a 
> mistake.

It aims to get rid of the ones that don't add any value.

> One more thing that I would point out is that your definition of
> DocParseException won't fly. file and line already exist in 
> Exception with
> different meanings, so you'd need different names for them in 
> DocParseException.

True.

Lars
April 02, 2013
Re: DIP33: A standard exception hierarchy
On Tuesday, 2 April 2013 at 10:37:08 UTC, deadalnix wrote:
> On Monday, 1 April 2013 at 11:08:16 UTC, Lars T. Kyllingstad 
> wrote:
>> It's time to clean up this mess.
>>
>> http://wiki.dlang.org/DIP33
>
> Several things.
>
> First the usage of enums isn't the right path. This makes it 
> hard to extend in general, and it is a poor man replacement for 
> sub classes in general.

Phobos/druntime devs can always add to the enums.  Users still 
have the option of subclassing if strictly necessary.


> As a rule of thumb, when you use switch in OOP code, you are 
> likely to do something wrong.

I'm not sure I agree with that rule.  And anyway, D's final 
switch mitigate some of the problems with classic switch.

> Second, many of you error are recoverable here. It isn't quite 
> satisfying.
>
> RangeError is a very bad thing IMO. It completely hides why the 
> range fails in the first place. Trying to access front when not 
> possible for instance, is an error for a reason (which is range 
> dependent). That reason must be the source of the 
> error/exception.

No.  To call front on an empty range is a programming error, 
plain and simple.  It's like trying to access the first element 
of an empty array.  The fact that some ranges may allow it anyway 
does not change anything.


> In general the hierarchy is weird. Why isn't 
> NetworkingException (why not NetworkException ?) a subclass of 
> IOException ?

Because they are supposed to signal different error conditions.


> OutOfMemoryError on its own isn't good IMO. The Error hierarchy 
> is made for error that aren't recoverable (or may not be 
> recoverable). It include a whole class of problem, and OOM is 
> only one of them (another example is Stack overflow errors).

The DIP sort of redefines Error to mean "programming error".

Lars
April 02, 2013
Re: DIP33: A standard exception hierarchy
On Tuesday, 2 April 2013 at 16:36:36 UTC, Jacob Carlborg wrote:
> On 2013-04-02 17:56, Lars T. Kyllingstad wrote:
>
>> In my experience, most of the time, you don't even bother 
>> distinguishing
>> between the finer categories.  If you can't open a file, well, 
>> that's
>> that.  Tell the user why and ask them to try another file.  (I 
>> realise
>> that this is highly arguable, of course.)
>
> I would say that there's a big difference if a file exist or if 
> you don't have permission to access it. Think of the command 
> line, you can easily misspell a filename, or forget to use 
> "sudo".

This illustrates my point nicely!  What does the shell do in this 
case?  It treats both errors the same:  It prints an error 
message and returns to the command line.  It does not magically 
try to guess the filename, find a way to get you permission, etc.

Lars
April 03, 2013
Re: DIP33: A standard exception hierarchy
On Tuesday, April 02, 2013 22:00:57 Lars T. Kyllingstad wrote:
> On Monday, 1 April 2013 at 23:03:56 UTC, Jonathan M Davis wrote:
> > On Monday, April 01, 2013 13:08:15 Lars T. Kyllingstad wrote:
> >> It's time to clean up this mess.
> >> 
> >> http://wiki.dlang.org/DIP33
> > 
> > Another concern I have is InvalidArgumentError. There _are_
> > cases where it
> > makes sense for invalid arguments to be an error, but there are
> > also plenty of
> > cases where it should be an exception (TimeException is
> > frequently used in
> > that way), so we may or may not want an
> > InvalidArgumentException, but if you
> > do that, you run the risk of making it too easy to confuse the
> > two, thereby
> > causing nasty bugs. And most of the cases where
> > InvalidArgumentError makes
> > sense could simply be an assertion in an in contract, so I
> > don't know that
> > it's really needed or even particularly useful. In general, I
> > think that
> > having a variety of Exception types is valuable, because you
> > catch exceptions
> > based on their type, but with Errors, you're really not
> > supposed to catch
> > them, so having different Error types is of questionable value.
> > That doesn't
> > mean that we shouldn't ever do it, but they need a very good
> > reason to exist
> > given the relative lack of value that they add.
> 
> I definitely don't think we need an IllegalArgumentException.
> IMO, passing illegal arguments is 1) a simple programming error,
> in which case it should be an Error, or 2) something the
> programmer can not avoid, in which case it requires a better
> description of why things went wrong than just "illegal
> argument". "File not found", for example.

If we had IllegalArgumentException, it would likely be the base class for some 
subset of exceptions which were always bad arguments, but it certainly 
wouldn't make sense to use it by itself in most cases. It would just provide a 
convenient way to catch that particular subset of exceptions if you needed to.

In general though, I think that assert covers what IllegalArgumentError is 
trying to do just fine, and where it doesn't, the argument about needing more 
descriptive exceptions applies just as well (e.g. RangeError). So, unless it's 
used as a base class for more descriptive errors, I don't think that there's 
much value in having IllegalArgumentError.

> I didn't really consider contracts when I wrote the DIP, and of
> course there will always be the problem of "should this be a
> contract or a normal input check?" The problem with contracts,
> though, is that they go away in release mode, which is certainly
> not safe if that is your error handling mechanism.

If you're treating Error exclusively as programming errors, then it's really 
no different from AssertError. You're just creating categories for specific 
types rather than using assert for them all. And I would fully expect things 
like RangeError to be compiled out in -release. That's what we've started 
doing in std.range and std.algorithm now that we've got version(assert). So, 
instead of having a function like opIndex assert, it checks and throws a 
RangeError on failure - but it does so in a version(assert) block so that it's 
compiled out. It (nearly) matches the behavior of arrays that way.

So, unless you're arguing that assertions should be left in code, then I don't 
think that it makes any sense to expect that Errors in general will stay in 
production code. Some may, depending on the code and what's being tested, but 
there's no guarantee that they will, and I think that it would be very much 
incorrect to expect them to in the general case - not when they equate 
specifically to programming errors.

- Jonathan M Davis
April 03, 2013
Re: DIP33: A standard exception hierarchy
On Tue, 02 Apr 2013 14:54:06 -0400, Jacob Carlborg <doob@me.com> wrote:

> On 2013-04-02 19:02, Steven Schveighoffer wrote:
>
>> Right, but what I see here is that the language uses one set of criteria
>> to determine whether it should catch, but it's difficult to use that
>> same criteria in order to process the exception.  It's not easy to
>> switch on a class type, in fact it's downright ugly (maybe we need to
>> come up with a way to do that in normal code too).
>
> That's kind of breaking the whole point of OO and virtual functions, you  
> should not need to know the exact type.

The example dictates we determine why the exception is thrown, yet that we  
can't catch the exact type, because we would have to duplicate the code  
block.

So we somehow have to catch the base type and then manually verify it's  
one of the types we want, and rethrow otherwise.  If there is a better  
idea, I'd love to hear it.

-Steve
April 03, 2013
Re: DIP33: A standard exception hierarchy
On Tuesday, 2 April 2013 at 19:10:47 UTC, Ali Çehreli wrote:
> > We can still throw exceptions in production, I don't tend to
> use this,
> > but maybe this would be a time to say "invalid state stop."
> But then how
> > do you distinguish it from "fix your program?"
>
> (I am not sure that I understand that comment correctly.)

(meant to say: we can still throw Errors in production.)

Errors currently really get used to identify conditions that 
could cause an invalid state as the check may not always be there.

> > I've mostly enjoyed having temporary files being cleaned up
> upon some
> > range access error which has no effect on my removing files
> that are no
> > longer valid.
>
> The problem is, the runtime cannot know that it will be doing 
> what you really wanted. The incorrect program state may result 
> in deleting the wrong file.

See above, the state isn't invalid. The error is thrown which is 
stating, "hey, buddy, good thing you didn't flip that release 
switch as I'm about to do something I shouldn't be."

However D does allow nothrow functions to throw Errors. I 
wouldn't say this would cause the program enter into an invalid 
state (memory corruption) but it would be a bad state (contract 
violations).

Take the RangeError thrown when you pop an empty range. Under 
what scenario would receiving one of these would indicate that my 
file isn't correct for deletion (any more so than say a 
ConvException from the same range).

    auto myFile = "some.tmp";
    scope(exit) remove(myFile);

    // setup code here
    manipulateFileRange(range);

setup code could of course assign a pointer of myFile to range 
and range could make modifications but doing so could just as 
likely throw a ConvException (or others) before you hit a 
RangeError.
4 5 6 7 8 9 10 11 12
Top | Discussion index | About this forum | D home