On Wednesday, 5 August 2015 at 02:52:40 UTC, Jonathan M Davis
wrote:
On Tuesday, 4 August 2015 at 22:13:40 UTC, Steven Schveighoffer
wrote:
On 8/4/15 5:39 PM, Jonathan M Davis wrote:
I'm certainly not opposed to have a message be printed before
the HLT
instruction with assert(0), but I don't at all agree that the
fact that
the message is not seen in -release is a reason not to have a
message.
For instance:
https://github.com/D-Programming-Language/druntime/blob/master/src/core/time.d#L2283
This makes it seem like a message will be printed in the case
where ticksPerSecond was 0. but in reality it simply throws a
segfault.
Whether this happens or not in debug mode is pretty much
irrelevant -- druntime is used in release mode by the vast
majority of all developers, and this passes unit tests for us.
It's the whole impetus for this thread, because someone
actually did find a case where it gets there.
So why have a message with the clock name that failed? Why not
just assert(0)? The only purpose I see for such a message is
to trick the reviewer into accepting it (not that this was the
intention of course) as being sufficiently explanatory when an
error occurs.
We should always review such code with the view that when it
*doesn't* print the message, is the error sufficient to a user
such that they know where to look. I find it hard to believe
it's *ever* sufficient, if you needed to have a message in the
first place.
We can look at it this way -- if you need to add a message to
an assert(0) for it to make sense, you should find a different
way to communicate that.
It was never expected that a user to see any of those messages
anyway. The idea was that if they failed, there was something
seriously wrong, and the program needed to be killed. If the
message prints when it fails, great. It's more explanatory that
way, but it's a bonus (and one that you do get if druntime is
build in debug mode). The purpose was to kill the program,
because it was in an invalid state.
Maybe in this case, it would have made more sense to simply throw
an Error rather than use assert(0), since it inadvertently ended
up depending on the system's environment, even though it was
supposed to be guaranteed to work (differences in kernel versions
was not taken into account). And I assume that if we threw an
Error, then we'd get a message.
- Jonathan M Davis