April 02, 2013 Re: DIP33: A standard exception hierarchy | ||||
---|---|---|---|---|
| ||||
Posted in reply to Jonathan M Davis | 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 | ||||
---|---|---|---|---|
| ||||
Posted in reply to Steven Schveighoffer | 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 | ||||
---|---|---|---|---|
| ||||
Posted in reply to Ali Çehreli | 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 | ||||
---|---|---|---|---|
| ||||
Posted in reply to Jesse Phillips | 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 | ||||
---|---|---|---|---|
| ||||
Posted in reply to Jonathan M Davis | 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 | ||||
---|---|---|---|---|
| ||||
Posted in reply to deadalnix | 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 | ||||
---|---|---|---|---|
| ||||
Posted in reply to Jacob Carlborg | 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 | ||||
---|---|---|---|---|
| ||||
Posted in reply to Lars T. Kyllingstad | 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 | ||||
---|---|---|---|---|
| ||||
Posted in reply to Jacob Carlborg | 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 | ||||
---|---|---|---|---|
| ||||
Posted in reply to Ali Çehreli | 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. |
Copyright © 1999-2021 by the D Language Foundation