Against enforce()

[Jonathan M Davis]

On Wednesday, March 16, 2011 16:45:56 bearophile wrote:
> enforce() seems the part of Phobos that I hate more. Among its faults:
> - It can't be used in weakly pure functions (because you can't use lazy
> arguments in pure functions), and not-purity is viral. Lot of Phobos can't
> be pure because of this, so even more user code can't be pure because of
> this; - It kills inlining (with the current DMD, and I don't think this
> problem will be fixed soon); - I have seen it slow down code (probably
> mostly because of its not-inlining nature);

> - Where it is used it usually
> doesn't give a more meaningful exception like WrongArgumentException, etc.
> I don't want a deep hierarchy of one hundred standard exceptions, but I
> think some standard exceptions for the most common mistakes, like wrong
> arguments, etc, are better than a generic enforce(), especially for a
> standard library code that is meant to be written with care and to give
> better error messages/exceptions.

A number of modules in Phobos have exceptions specific to that module - such as 
DateTimeException or FileException. enforce doesn't stop anyone from using 
specific exceptions. And it's just as easy to use Exception instead of a specific 
exception type when throwing directly, so I don't think that this complaint 
holds much water. It's a valid complaint if Phobos developers are choosing to 
throw Exception instead of more specific exceptions, but that's not enforce's 
fault.

> - It doesn't allow functions to be
> nothrow. This is a fault, because D has Contract Programming, that is
> meant to be usable for nothrow functions too. D Contracts with asserts are
> the right tool.

Overall, I favor exceptions when it comes to testing input rather than 
assertions, but regardless of that. enforce does _not_ stop functions from being 
nothrow. It just makes it more annoying to make them nothrow. If you want the 
caller to be nothrow, you have to have a try-catch block in the caller which 
catches Exception. If the function really isn't ever going to throw, then you 
can use such a try-catch block with an assert(0) in the catch body, and it's 
quite safe. std.datetime does that in several places.

Regardless, your problem here really isn't enforce anyway. Your problem is that 
you think that a number of functions should be using assertions instead of 
exceptions. enforce happens to be a way to throw exceptions, but it's quite easy 
to use exceptions without it. So, enforce doesn't really have anything to do 
with it other than the fact that that was the means used to throw the exception.

> I see enforce() just as a temporary workaround for a problem of Phobos
> (that it's compiled in release mode, so its asserts are vanished) that
> risks to become a permanent part of Phobos.

> So a better solution is for the standard Phobos library to ship in two
> versions, one compiled in release and not release mode, and DMD may choose
> the right one according to the compilation switches. This removes most of
> the need of enforce(). I suggest to deprecate enforce(). Until the problem
> with Phobos compilation is solved and enforces are removed from Phobos,
> enforce() may become a private Phobos function that user code can't
> import.

There a number of places in Phobos which throw exceptions and _should_ throw 
exceptions. ConvExceptions and FileExceptions are great examples. 
DateTimeException is used liberally in std.datetime, and it really wouldn't make 
sense to make them use assertions.

And even in cases where exceptions were chosen of assertions because of the 
release vs non-release mode issue, there's still the question of whether we 
really want Phobos to be throwing AssertErrors, since when a programmer sees an 
assertion which isn't theirs, they're likely to think that it's someone else's 
code which is broken, not theirs. So, having AssertErrors thrown from Phobos 
look just plain bad. Now, in same cases performance still makes assertions more 
desirable - and there _are_ places in Phobos which use exceptions - but there 
are still a lot of situations where Phobos _should_ be throwing exceptions.

Regardless, enforce is supposed to make throwing exceptions similar to asserting 
something. It is - in theory at least - a great idea. The problem with it is its 
laziness and all of the implications that that has (like the inability to inline 
because of it). If we had a non-lazy version of enforce or if the compiler were 
made smart enough to realize that it didn't need a lazy version in some cases 
(such as when it's passed a string literal) and to use a non-lazy version in 
such cases, then the problem would be reduced.

Personally, I rarely use enforce precisely because of the inlining issue. I 
don't that that

if(!condition)
    throw new ExceptionType(msg);

is really much worse than

enforce(condition, new ExceptionType(msg));

but I see no reason to get rid enforce. I also disagree with you about how much 
Phobos should be using exceptions. There _are_ places - such as in a lot of 
range-based functions - which likely need to use assertions rather than 
exceptions for performance reasons, but I do _not_ like the idea of using 
assertions to validate input from code outside of Phobos. I think that that is 
_exactly_ the sort of place that should be using exceptions rather than 
assertions.

- Jonathan M Davis