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

Reply via email to