try/catch idiom in std.datetime

[Jonathan M Davis]

On Monday, November 18, 2013 00:14:24 Andrei Alexandrescu wrote:
> On 11/17/13 11:28 PM, Jonathan M Davis wrote:
> > On Sunday, November 17, 2013 22:32:46 Andrei Alexandrescu wrote:
> "Too much of a good thing" comes to mind (as does with the massive and
> repetitive unittests in std.datetime). If code looks and feels meh, it
> probably is. Just apply good judgment instead of rote adherence to rules.

Well, my coding style has adjusted over time, and I'm tending to put fewer 
blank lines in cases like this, precisely because it's not terribly pretty.

> as does with the massive and repetitive unittests in std.datetime

Those are quite valuable IMHO, as being thorough has caught a lot of problems, 
and as we've discussed (and disagreed on) before, I prefer unit tests to be 
dead simple in order to reduce the odds of them being buggy, even if that 
means that they're more verbose. I have done some work however to move the 
tests toward using loops instead in order to reduce the number of lines of 
code, but I have quite a bit still to do there. I'll probably get back to that 
after I finish with splitting std.datetime, but I haven't finished that yet 
because of how busy I've been the past few months. So, everything takes 
forever... Oh well, I should get back to that soon enough.

> >> 0. Do nothing. This is as good as it gets.
> > 
> > Well, it works just fine as-is, but it would be kind of nice to be able to
> > solve the problem in a less verbose manner (though you're talking about
> > saving only a few lines of code).
> 
> I'm also concerned about generated code size and overall efficiency. It
> looks like the assert(0) insertions are there simply to validate the
> design (they only fail if Phobos has an internal error), so there should
> be some means to remove them in release builds. We don't have such a
> possibility at the moment.

Well, the assertions are there to validate the design, but the try-catches are 
required to make the function nothrow, and I'd feel very funny having an empty 
catch block, though having it compiled out in -release wouldn't necessarily be 
a bad idea (which could probably be done if a function which returned false 
where used in the assertion instead of a constant).

> >> 2. Relax the nothrow guarantees. After all, nothrow is opt-in.
> > 
> > I'm not quite sure what you're suggesting here.
> 
> What I meant is, not everything that is nothrow needs to be annotated as
> such.

So, you're suggesting that nothrow be inferred?

> >       @property FracSec fracSec() const nothrow
> >       {
> >       
> >           trusted-nothrow
> >           {
> >           
> >               auto hnsecs = removeUnitsFromHNSecs!"days"(adjTime);
> >               
> >               if(hnsecs < 0)
> >               
> >                   hnsecs += convert!("hours", "hnsecs")(24);
> >               
> >               hnsecs = removeUnitsFromHNSecs!"seconds"(hnsecs);
> >               
> >               return FracSec.from!"hnsecs"(cast(int)hnsecs);
> >           
> >           }
> >       
> >       }
> 
> That can be done at library level.

Well, it is being done at the library level right now via try-catch.

> >> 4. ...?
> > 
> > We now have std.exception.assumeWontThrow, which works reasonably well
> > when
> > you need to wrap a single call as opposed to several, but it has the same
> > problem as enforce in that it uses lazy, which is definite performance
> > hit. So, in most cases, I'd be more inclined to just use a try-catch, and
> > if it's more than one expression, you pretty much need to use try-catch
> > or scope(failure) instead anyway, since you wouldn't want to wrap whole
> > function bodies in a call to assumeWontThrow (assuming that you even
> > could).
> 
> Again, one problem here (in addition to the blowup in code size and
> decay of readability) is we're talking about an internal design
> validation, not meaningful runtime semantics. There should be a way to
> remove the baggage.

I think that that would require a language change of some kind, because it 
would require that the compiler recognize that the check can go away in
-release, which would effectively mean having the try-catch block be optimized 
away somehow when compiling with -release. Using scope(failure) for that 
wouldn't change anything, as it would just mean that the compiler was 
inserting the try-catch statements for you. It would just be slightly less 
verbose.

Maybe the compiler can be changed to optimize out try-catch blocks if the 
catch block just contains an assertion? That would at least get rid of the 
overhead of the idiom, even if it didn't make it less verbose. If we can't get 
the compiler to recognize the idiom and optimize out the unnecessary code, 
then if we want it to be optimized out, I think that we'll be forced to add 
something to the language which the compiler _does_ recognize and therefore 
can compile out when appropriate - unless someone can come up with some great 
idea for improving nothrow, but that seems a lot like it would be moving 
towards not needing @trusted anymore, because the compiler would figure that 
out for you. I don't see how you could get away from having to have the 
programmer tell the compiler that they're sure that the function which can 
throw won't throw inside the nothrow function, because that requires human 
judgement just like @trusted does, though it should generally be easier for a 
programmer to be sure that something won't throw than it is to be sure that 
it's actually @safe.

- Jonathan M Davis