[blocker fix]: use auto return in std.datetime

[WalterBright]

This is a blocker for dlang/dmd#4136

Without it, the error is:

std\datetime.d(17341): Error: function std.datetime.Interval!(TimeOfDay).Interval.end need 'this' to access member end
std\datetime.d(17374):        called from here: end()
std\datetime.d(18699): Error: template instance std.datetime.Interval!(TimeOfDay) error instantiating
std\datetime.d(17341): Error: function std.datetime.Interval!(DateTime).Interval.end need 'this' to access member end
std\datetime.d(17374):        called from here: end()
std\datetime.d(18700): Error: template instance std.datetime.Interval!(DateTime) error instantiating
std\datetime.d(18685): Error: pure nested function '__invariant63' cannot access mutable data '_begin'
std\datetime.d(18685): Error: need 'this' for '_begin' of type 'TimeOfDay'
std\datetime.d(18685): Error: pure nested function '__invariant63' cannot access mutable data '_end'
std\datetime.d(18685): Error: need 'this' for '_end' of type 'TimeOfDay'

[quickfur]

LGTM.

[quickfur]

Auto-merge toggled on

[MartinNowak]

The change doesn't make the code worse, but we shouldn't change legit code to let dmd pass the testsuite.

[mihails-strasuns]

Wait, what? We have just changed Phobos to ignore DMD regression that will bite us upon next release?

[WalterBright]

It's not a dmd regression. If you add the following to the class as line 14219:

    public pure nothrow @safe @nogc ~this() { }

it will fail with previous dmd in the same way. I.e. it's an existing problem.

[mihails-strasuns]

How is this related?

What I see that there is a simple line of code in Phobos that did compile before and stopped working with dmd change. Same code pattern can possibly exist in user code and will stop compiling too. Or am I missing something?

[WalterBright]

What the dmd change was was it added a destructor so that the invariant would run upon destruction of the object. Adding the destructor manually produces the same effect.

[WalterBright]

https://issues.dlang.org/show_bug.cgi?id=13721

[jmdavis]

But shouldn't typeof(end - begin) and auto be returning the same type? I don't know why it didn't return auto originally (probably a compiler bug that existed at the time the code was written that doesn't exist now), but as far as I can tell, both versions of the code should compile, and this certainly comes across as changing the code simply to mask a compiler bug. And I concur with Dicebot in that I don't see how adding a destructor is relevant. Even if doing so would trigger the same bug, the fact that typeof(end - begin) worked before but doesn't now (regardless of what bug it now triggers) means that we have a regression, and this change hides it (even if it would otherwise be a good change). So, from that perspective, this seems like a bad change.

[dnadlinger]

@WalterBright:

What the dmd change was was it added a destructor so that the invariant would run upon destruction of the object. Adding the destructor manually produces the same effect.

So your reasoning is: "There was a way to explicitly trigger the same bug before, so it's not a regression"? That's not what users care about at all.

[MartinNowak]

Let's agree on the following. It's a separate compiler bug that was revealed by adding a dtor to a struct.
Until the next dmd release we'll have to fix it, because otherwise adding dtors for invariants will cause regressions by triggering bugs (I've raise the importance of the bug to blocker).
OTOH I understand that Walter wants to go on, because tracing down every compiler bug during development usually leads to endless recursion.

[dnadlinger]

Sure, this is a sensible thing to do. It's the claim that it is not a (potential) regression I objected to.

[MartinNowak]

It's the claim that it is not a (potential) regression I objected to.

Yeah, it's indeed something we'll have to fix soonish.