On Tuesday, 4 August 2015 at 21:07:20 UTC, Steven Schveighoffer wrote:
> On 8/4/15 4:56 PM, Jonathan M Davis wrote:
>> On Tuesday, 4 August 2015 at 20:48:52 UTC, Steven Schveighoffer wrote:
>>> On 8/4/15 4:43 PM, Steven Schveighoffer wrote:
>>>> I should say, any assert(0) with a message printed that is possible to
>>>> trigger in release mode is an error. If you can ensure it's only
>>>> possible to trigger when compiled in debug mode (or for unit tests),
>>>> then assert(0, msg) is fine.
>>>
>>> In that vein, would it be possible to make this a warning? i.e. if you
>>> compile a file in -release mode, and the compiler encounters an
>>> assert(0, msg), it emits a warning?
>>
>> I fail to see why it's a problem if assert(0) has a message. It's
>> exactly the same as any other assertion except that it stays even with
>> -release, and you don't get the message then (whereas with a normal
>> assertion, you wouldn't get anything).
>
> Seeing an assert with a message gives the impression that you will see the message if you get there. I get that normal asserts may not even trigger. But the concept of "oh, if I see that message I know what happened" really goes out the window if you are never going to see it.
>
> As I said earlier, any line in druntime that has an assert(0, msg) is a bug, because the message is never seen.

It stills indicate why the assertion is there, which can be helpful to maintainers, and it _is_ possible to build druntime in debug mode. It's just not released that way (though it's been argued before that druntime/Phobos should be released with both debug and release builds).

And arguing against assert(0) with a message in druntime because it's not going to show the message normally would basically be the same as arguing against having any assertions in there at all, because they wouldn't show up in a typical build either. But that doesn't mean that they shouldn't be there, just that they're not as useful as they might otherwise be, because druntime is almost always built in release mode.

> Doing the static if thing isn't the right answer either. The right answer is to choose one or the other (and by choosing to print a message, you could either throw an assert error directly, or print the message specifically and then assert(0) ).
>
> So basically:
>
> assert(0, msg);
>
> becomes
>
> printSomehow(msg);
> assert(0);
>
> With druntime it's difficult to actually print the message (and my PR is trying to fix that).

I really so no difference between having a message with an assert(0) as with any other assertion. You're not going to see either with -release. It's just that assert(0) kills your program instead of giving you undefined behavior.

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.

- Jonathan M Davis

On Tuesday, 4 August 2015 at 20:21:24 UTC, Walter Bright wrote:
> On 8/3/2015 8:37 PM, Ola Fosheim =?UTF-8?B?R3LDuHN0YWQi?= <ola.fosheim.grostad+dlang@gmail.com> wrote:
>> The input/environment/code distinction does not work very well.
>
> Sure it does. If your user ever sees an assert failure message, your program has a bug in it.

Yes.

> Keep that in mind when designing the code, and the distinction will become clear.

The input/code distinction is too simplistic.

Example 1: It makes perfect sense to assert (or assume) that a value from a hardware register or cpu instruction is within range. If the assert fires, it is the spec/code that is wrong, not the input. So you are testing the specification in the code against well defined input.

Example 2: It makes perfect sense to enforce that a return value from a plugin library is within range to maintain main program integrity and shut out a misbehaving plugin.

And so on.

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.

-Steve

On Tuesday, 4 August 2015 at 22:06:06 UTC, Ola Fosheim Grøstad wrote:
> On Tuesday, 4 August 2015 at 20:21:24 UTC, Walter Bright wrote:
>> On 8/3/2015 8:37 PM, Ola Fosheim =?UTF-8?B?R3LDuHN0YWQi?= <ola.fosheim.grostad+dlang@gmail.com> wrote:
>>> The input/environment/code distinction does not work very well.
>>
>> Sure it does. If your user ever sees an assert failure message, your program has a bug in it.
>
> Yes.
>
>> Keep that in mind when designing the code, and the distinction will become clear.
>
> The input/code distinction is too simplistic.
>
> Example 1: It makes perfect sense to assert (or assume) that a value from a hardware register or cpu instruction is within range. If the assert fires, it is the spec/code that is wrong, not the input. So you are testing the specification in the code against well defined input.
>
> Example 2: It makes perfect sense to enforce that a return value from a plugin library is within range to maintain main program integrity and shut out a misbehaving plugin.
>
> And so on.

I'm 90% on Walter's side here as he's right for the majority of common cases, but on the other hand there are plenty of situations where the boundary between code error and environment error get blurred.

On 8/4/2015 1:56 PM, Jonathan M Davis wrote:
> I can definitely see an argument that we should be
> printing out something before the HLT instruction in -release mode,

    writeln("Somebody forgot to turn off the water.");
    assert(0);

On 8/4/2015 2:07 PM, Steven Schveighoffer wrote:
> With druntime it's difficult to actually print the message

    fprintf(stderr, "You left the headlights on.");
    assert(0);

On 8/4/2015 3:13 PM, Steven Schveighoffer wrote:
> 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.

That's a bug in core/time.d, NOT a bug with asserts.

On 8/4/2015 4:37 PM, Walter Bright wrote:
> On 8/4/2015 3:13 PM, Steven Schveighoffer wrote:
>> 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.
>
> That's a bug in core/time.d, NOT a bug with asserts.
>

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

On Tuesday, 4 August 2015 at 23:33:17 UTC, Walter Bright wrote:
> On 8/4/2015 2:07 PM, Steven Schveighoffer wrote:
>> With druntime it's difficult to actually print the message
>
>     fprintf(stderr, "You left the headlights on.");
>     assert(0);

It seems that on american car, you bip as annoyingly as possible instead of announcing what's wrong. Preferably for anything and its reverse.

On 8/4/2015 4:56 PM, deadalnix wrote:
> It seems that on american car, you bip as annoyingly as possible instead of
> announcing what's wrong. Preferably for anything and its reverse.

It turns out you can make quite understandable speech simply by connecting an on/off I/O port to a speaker. So there is no excuse for using tones instead of "You left the headlights on, you ******* ****!"