On Wednesday, 3 April 2013 at 00:07:10 UTC, Jonathan M Davis wrote:
> [...]
> 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.

I disagree.  Some things don't really deserve a more detailed error than just "illegal argument".  Examples include passing a negative number where a positive one was expected, an invalid combination of bit flags, an invalid file mode to a file-opening function, etc.


> 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). [...]

The problem with assert is that it gets disabled in release mode.  I think it is a bad idea to have this be the "standard" behaviour of parameter validation.


> [...]
>
> 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.

Wouldn't you say that most of the Error types we have today do, in fact, signal programming errors?  Walter even argued that OutOfMemoryError is often a programming error.


> [...]
>
> 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. [...]

I personally think that, as a general rule, Errors should stay in production code.  I thought we had already separated -noboundscheck from -release, but I just tested now and that doesn't seem to be the case: -release still implies -noboundscheck.

D prides itself on being safer than C and C++, what with its default array bounds checks to combat buffer overruns and all, but as it turns out we're not that much better off.  The -release switch practically screams "use me in production code", but what's the point of bounds checking if it is only ever used while developers are testing their code?

Lars

On Wednesday, 3 April 2013 at 05:42:13 UTC, Lars T. Kyllingstad wrote:
> The problem with assert is that it gets disabled in release mode.
>  I think it is a bad idea to have this be the "standard" behaviour of parameter validation.

Allow me to clarify my position on assert() a bit.  As a general rule, I think assert() should be used to verify internal program flow and program invariants, and nothing more.  Specifically, public APIs should *not* change depending on whether asserts are compiled in or not.

Say I am writing a function that you are using.  I don't trust you to always give me correct parameters, so I check them.  (Maybe my function could even do something dangerous if I didn't.)

  public void myFunction(someArgs)
  {
      if (someArgs are invalid)
          throw new InvalidArgumentError;
      ...
  }

Now that I have verified that your input is correct, i.e. that your part of the deal is done, everything that happens afterwards is entirely up to me to get right.  And *that* is when assertations enter the picture.  I'll sprinkle my code with assert statements to make sure that every step of whatever procedure myFunction() performs is correct, and I may use it to verify input parameters to *private* helper functions and such.  These kinds of checks are purely for my personal use, for debugging and for verifying that changes I make do not break other parts of the code.

There may of course be situations, e.g. in performance-critical code, where it is desirable to disable parameter validation, but then -release should not be the switch that does it.
-version=TotallyUnsafeButVeryPerformantDudeYourAeOnYourOwnNow, anyone? :)

Lars

On 4/2/13, Jesse Phillips <Jessekphillips+d@gmail.com> wrote:
> The spec makes no such guarantee

Yeah but let's not treat the spec as if it's something that was written professionally by an ISO committee. :)

> DMD just doesn't bother to not
> execute it. It is one of the many things which will break code
> when DMD decides to follow the spec.

I was wondering about this recently, and thought it's rather unsafe that Errors trigger finally blocks.

Anyway is this bug filed somewhere? We don't want to lose track of this.

On 2013-04-03 05:14, Steven Schveighoffer wrote:

> 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.

My objection was mostly to "maybe we need to come up with a way to do that in normal code too". Exceptions are kind of weird in that you break what OO is all about, not having to know the exact type. You may not need that with exceptions either, depending on what you do with them.

-- 
/Jacob Carlborg

On 2013-04-02 22:15, Lars T. Kyllingstad wrote:

> 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.

No, but you do know the difference. It doesn't just say "can't open file <filename>". It will say either, "file <filename> doesn't exist" or "don't have permission to access <filename>". It's a huge difference. I know _what_ went wrong with that file, not just that _something_ when wrong.

-- 
/Jacob Carlborg

On Wednesday, 3 April 2013 at 07:01:09 UTC, Jacob Carlborg wrote:
> On 2013-04-02 22:15, Lars T. Kyllingstad wrote:
>
>> 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.
>
> No, but you do know the difference. It doesn't just say "can't open file <filename>". It will say either, "file <filename> doesn't exist" or "don't have permission to access <filename>". It's a huge difference. I know _what_ went wrong with that file, not just that _something_ when wrong.

Which is exactly what you'd use FilesystemException.kind and/or FilesystemException.msg for.

I never said there shouldn't be a way to distinguish between file errors, I just said that in most cases, an entirely new exception class is overkill.

Lars

On Tuesday, 2 April 2013 at 20:11:31 UTC, Lars T. Kyllingstad wrote:
> 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.
>

It is illegal for a reason. For instance, with an array, it is an out of bound access. I see ne benefice to hide this information in a more generic RangeError. This is hiding information and providing nothing more.

On Tuesday, 2 April 2013 at 20:11:31 UTC, Lars T. Kyllingstad wrote:
> Phobos/druntime devs can always add to the enums.  Users still have the option of subclassing if strictly necessary.
>

This is fundamentally incompatible with :

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

As adding an entry unto the enum will break every single final switch in user code.

On 4/2/2013 1:00 PM, Lars T. Kyllingstad wrote:
> 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.

A bit of philosophy here:

Contracts are not there to validate user input. They are only there to check for logic bugs in the program itself.

It's a very clear distinction, and should not be a problem.

To reiterate, if a contract fails, that is a BUG in the program, and the program is then considered to be in an undefined state and is not recoverable.

CONTRACTS ARE NOT AN ERROR HANDLING MECHANISM.

On 4/2/2013 10:42 PM, Lars T. Kyllingstad wrote:
> The -release switch practically screams "use me in
> production code", but what's the point of bounds checking if it is only ever
> used while developers are testing their code?

For one thing, you can turn it on and off and actually measure how much the extra checks are costing.

For another, it's up to the programmer how he wants to play it.