On Monday, 3 August 2015 at 23:57:36 UTC, Steven Schveighoffer wrote:
> OK, this brings up another debate. The thing that triggered all this is an issue with core.time, see issue https://issues.dlang.org/show_bug.cgi?id=14863
>
> [...]

Why not just " stderr.writeln(errMsg); assert(0);"?

On 8/3/2015 4:57 PM, Steven Schveighoffer wrote:
> OK, this brings up another debate. The thing that triggered all this is an issue
> with core.time, see issue https://issues.dlang.org/show_bug.cgi?id=14863
>
> Essentially, we wrote code to get all the clock information at startup on a
> posix system that supports clock_gettime, which is immutable and can be read
> easily at startup. However, how we handled the case where a clock could not be
> fetched is to assert(0, "don't support clock " ~ clockname).
>
> The clear expectation was that the message will be printed (along with file/line
> number), and we can go fix the issue.
>
> On Linux 2.6.32 - a supported LTS release I guess - one of these clocks is not
> supported. This means running a simple "hello world" program crashes at startup
> in a segfault, not a nice informative message.
>
> So what is the right answer here?

The answer is the code is misusing asserts to check for environmental errors. I cannot understand how I consistently fail at explaining this. There have been many thousand message threads on exactly this topic.

Asserts are for CHECKING FOR PROGRAM BUGS.

enforce(), etc., are for CHECKING FOR INPUT ERRORS. Environmental errors are input errors, not programming bugs.

On 8/3/2015 5:25 PM, Ali Çehreli wrote:
> On 08/03/2015 04:57 PM, Steven Schveighoffer wrote:
>
>  > At the very least, assert(0, "message") should be a compiler error, the
>  > message is unused information.
>
> Agreed.

No.

1. If you want the message, do not use -release.
2. Do not use asserts to issue error messages for input/environment errors.

On Monday, 3 August 2015 at 15:18:12 UTC, Dicebot wrote:
> On Monday, 3 August 2015 at 14:34:52 UTC, Steven Schveighoffer wrote:
>> Why do we do this?
>
> Because all asserts must be completely removed in -release
>
> Yet assert(0) effectively mean "unreachable code" (it is actually defined that way in spec) and thus it is possible to ensure extra "free" bit of safety by crashing the app.

It isn't free. The point of having unreachable as primitive is that you can remove all branching that leads to it.

On Tuesday, 4 August 2015 at 02:37:00 UTC, Ola Fosheim Grøstad wrote:
> On Monday, 3 August 2015 at 15:18:12 UTC, Dicebot wrote:
>> On Monday, 3 August 2015 at 14:34:52 UTC, Steven Schveighoffer wrote:
>>> Why do we do this?
>>
>> Because all asserts must be completely removed in -release
>>
>> Yet assert(0) effectively mean "unreachable code" (it is actually defined that way in spec) and thus it is possible to ensure extra "free" bit of safety by crashing the app.
>
> It isn't free. The point of having unreachable as primitive is that you can remove all branching that leads to it.

"free compared to throwing exception" would be a better wording indeed. I remember Iain complaining about presence of HLT (and saying he won't do that for gdc) for exactly this reason.

On Tuesday, 4 August 2015 at 03:04:14 UTC, Dicebot wrote:
> indeed. I remember Iain complaining about presence of HLT (and saying he won't do that for gdc) for exactly this reason.

Yes. I think a lot of this confusion could have been avoided by using "halt()" instead of "assert(0)" and "unreachable()" for marking code that has been proven unreachable. Right now it isn't obvious to me as a programmer what would happen for "assert(x-x)". I would have to look it up.

A more principled approach would be:

- "enforce(...)" test for errors

- "assert(...)" are (lint-like) annotations that don't affect normal execution, but can be requested to be dynamically tested.

- "halt()" marks sudden termination

- "unreachable()" injects a proven assumption into the deductive database

-"assume(...)" injects a proven assumption into the deductive database

- contracts define interfaces between separate pieces of code (like a plugin or dynamically linked library from multiple vendors e.g. sub system interface) that you may turn on/off based on what you interface with.

The input/environment/code distinction does not work very well. Sometimes input is a well defined part of the system, sometimes input is code (like dynamic linking a plugin), etc...

On 8/3/15 9:13 PM, Walter Bright wrote:
> On 8/3/2015 5:25 PM, Ali Çehreli wrote:
>> On 08/03/2015 04:57 PM, Steven Schveighoffer wrote:
>>
>>  > At the very least, assert(0, "message") should be a compiler error,
>> the
>>  > message is unused information.
>>
>> Agreed.
>
> No.
>
> 1. If you want the message, do not use -release.
> 2. Do not use asserts to issue error messages for input/environment errors.
>

Any assert(0) in druntime or phobos with a message printed is an error. It's compiled in release mode.

I'll look through and find them, and rework them.

-Steve

On 8/3/15 9:11 PM, Walter Bright wrote:

> enforce(), etc., are for CHECKING FOR INPUT ERRORS. Environmental errors
> are input errors, not programming bugs.

enforce isn't available in druntime. In this case, we don't want to throw an exception anyways, or we would have to remove nothrow from core.time. Technically, usage of this unsupported clock will ALWAYS throw, or NEVER throw. That's difficult to encode into the system.

The only answer I can think of is to write a message to stderr, and then assert(0) as meta suggested.

-Steve

On 08/03/2015 11:59 AM, Dicebot wrote:
> General advice  - simply don't ever use -release unless you are _very_
> sure about program correctness (to the point of 100% test coverage and
> previous successful debug runs)

This is very true. I never disable asserts or bounds checking for exactly that reason - you can NEVER conclusively determine through prerelease testing that none of those conditions are going to get tripped in real-world usage. ANY developer who thinks they can is absolutely fooling themself. And what happens for the end user WHEN one of those conditions does occur? Memory corruption or otherwise invalid state. Things go boom. Whee. BAD idea.

There is only ONE time when asserts or bounds checking should EVER be disabled and that's on a per-function basis (split it out into a separate module if you need to) AFTER profiling has determined that specific location to be a significant bottleneck, and the code in question has been (and will continue to be during all future maintenance) VERY carefully combed-over and peer-reviewed to ensure (as much as possible) that disabling asserts/bounds checks on that localized function cannot lead to corruption, exploits or invalid state.

On Tuesday, 4 August 2015 at 14:40:16 UTC, Nick Sabalausky wrote:
> On 08/03/2015 11:59 AM, Dicebot wrote:
>> General advice  - simply don't ever use -release unless you are _very_
>> sure about program correctness (to the point of 100% test coverage and
>> previous successful debug runs)
>
> This is very true. I never disable asserts or bounds checking for exactly that reason - you can NEVER conclusively determine through prerelease testing that none of those conditions are going to get tripped in real-world usage. ANY developer who thinks they can is absolutely fooling themself. And what happens for the end user WHEN one of those conditions does occur? Memory corruption or otherwise invalid state. Things go boom. Whee. BAD idea.
>
> There is only ONE time when asserts or bounds checking should EVER be disabled and that's on a per-function basis (split it out into a separate module if you need to) AFTER profiling has determined that specific location to be a significant bottleneck, and the code in question has been (and will continue to be during all future maintenance) VERY carefully combed-over and peer-reviewed to ensure (as much as possible) that disabling asserts/bounds checks on that localized function cannot lead to corruption, exploits or invalid state.

Recently we had quite a lengthy discussion at work regarding possible guidelines for using asserts, contracts and enforce (we have similar own implementation) that would actually allow using -release flag for release builds. And got to certain principles that I believe may work in practice (even though they violate DbC ideology). I will check if I can publish those here.