On 7/9/17 7:45 AM, Nicholas Wilson wrote:
> On Sunday, 9 July 2017 at 11:37:55 UTC, Steven Schveighoffer wrote:
>> On 7/9/17 7:00 AM, Walter Bright wrote:
>>> On 7/9/2017 3:37 AM, Steven Schveighoffer wrote:
>>
>>>> Yet, here is an example of where we have effectively added a null pointer exception. > At the very least, this should be eliminated on Linux
>>>> and just use the signal handling null pointer error mechanism!
>>>
>>> You're a few years late, as pretty much nobody agreed with me that the operating system handling of it was plenty.
>>
>> I think you misunderstand, we have etc.linux.memoryerror that can actually throw an error on a null pointer using the signal handler.
>>
>> I have a suggestion: eliminate this feature, and add a -npe switch to the compiler that errors on any null pointer usage. Asserts will be sprinkled in everywhere, but may be useful to someone debugging a nasty null pointer segfault somewhere.
> 
> I think the generated assert(this !is null) has its place, it is useful to catch a null this as early as possible but not by default. Perhaps debug mode (as in the compiler switch) or a switch of its own.

I'd argue it's not useful at all. I've seen segfaults many many many times when debugging D code. I've never seen this error show up. Even when developing RedBlackTree (which is full of null pointers to structs on every leaf). And it makes sense why too:

1. Structs are generally allocated on the stack, or an array, or inside another type. Very rarely would you have a struct pointer that you didn't initialize (and was therefore null).
2. Often times, you are using a struct's data members, so you get a segfault before ever trying to call a method on it.
3. Classes are where you might see this issue, as people declare a class and try to use it without allocating one all the time. But in this case, when you are calling a virtual function, the segfault occurs before the assert can ever be used.

That being said, if people depend on it for some reason, switching it to an opt-in feature would be fine with me. In that case, I suggest just going whole-hog, and instrumenting all pointers.

-Steve

On 09.07.2017 13:57, Steven Schveighoffer wrote:
> 3. Classes are where you might see this issue, as people declare a class and try to use it without allocating one all the time. But in this case, when you are calling a virtual function, the segfault occurs before the assert can ever be used.

What about final member functions?

On 7/9/17 8:04 AM, Timon Gehr wrote:
> On 09.07.2017 13:57, Steven Schveighoffer wrote:
>> 3. Classes are where you might see this issue, as people declare a class and try to use it without allocating one all the time. But in this case, when you are calling a virtual function, the segfault occurs before the assert can ever be used.
> 
> What about final member functions?

It's possible one has a final class, and does experience this. But it's not likely -- most people don't think about virtual by default, and just have virtual functions.

Or the final member functions are called after attempting to access members or virtual functions.

Or you just learn not to leave class instances uninitialized (most have), and you never see the error.

I'll also note that dmd 2.050 still segfaults even for final functions. 2.060 doesn't. I didn't test in between. So at least as far back as 2.050, this was a wasted assert.

-Steve

On Sunday, July 9, 2017 4:00:33 AM MDT Walter Bright via Digitalmars-d wrote:
> On 7/9/2017 3:37 AM, Steven Schveighoffer wrote:
> > Wait, you have stated many many times, a segfault is good enough, it's not worth the added cost to do null pointer exceptions (a position I'm completely in agreement with).
>
> That's right.
>
> > Yet, here is an example of where we have effectively added a
> > null pointer exception. > At the very least, this should be eliminated
> > on Linux and just use the signal handling null pointer error mechanism!
>
> You're a few years late, as pretty much nobody agreed with me that the operating system handling of it was plenty.

What I don't understand about this is that it's not like we have these sort of checks in general - just in this weirdly specific case. I could understand that argument if we were doing null pointer checks in general, but we're not, and you clearly haven't given in to the push for that.

In _C++_, I have literally only seen a null this pointer inside a member function twice in my career (and the second time it happened, I had to explain what was happening to my coworkers - some of them being quite knowledgeable - because they didn't even think that it was possible to call a function with a null pointer and not have it blow up at the call site). I have never seen this problem in D. I would be _very_ surprised if you couldn't just remove this check, and no one would complain because they hit this problem and didn't get an assertion.

> > Note that there is a significant difference between this situation
> > (where you are *adding* an extra check), and the argument to add
> > messages to asserts (where you are *already* asserting).
>
> It's not really different. It's the desire for ever more messages. I've long advocated that a file/line is quite sufficient, but I seem to be in a tiny minority of 1. Now 2. :-)

For some assertions, having more than a file and line number is nice (e.g. if you have messages for your pre-condition assertions, then you can make it so that when it fails, the programmer doesn't even need to look at the source code), but for many, many assertions, having a message doesn't do much for you IMHO. You need to look at the code anyway, and if it's asserting an internal thing rather than DbC, then it's usually really not the sort of thing where a message is going to help particularly. In such cases, the only real benefit that I see from having an error message that does more than tell you where the assertion was and what the call stack was is that if you have a message, and there are several assertions in the same area of code, then when an assertion fails, you're less likely to mistake one assertion for another if the source you're looking at doesn't exactly match the build that the person reporting the issue was using (and that mismatch isn't always obvious if you're not just building and running your code locally).

So, to an extent at least, I agree with you, but a number of folks do seem to think that messages that add no information are better than no messages for some reason, and unfortunately, there's now a PR to outright require messages for all assertions in Phbos:

https://github.com/dlang/phobos/pull/5578

- Jonathan M Davis

https://github.com/dlang/dmd/pull/6982

On 7/9/2017 6:37 PM, Jonathan M Davis via Digitalmars-d wrote:
> For some assertions, having more than a file and line number is nice (e.g.
> if you have messages for your pre-condition assertions, then you can make it
> so that when it fails, the programmer doesn't even need to look at the
> source code),

He always needs to look at the source code. Asserts are for BUGS, and to fix bugs, you gotta look at the code. If asserts are being used as a substitute for input/environment errors, they're misused.

In 40 years of asserts, I can't think of ever not visiting the file/line where it failed as the first thing I did to debug it.

On 7/9/2017 4:37 AM, Steven Schveighoffer wrote:
> On 7/9/17 7:00 AM, Walter Bright wrote:
>> On 7/9/2017 3:37 AM, Steven Schveighoffer wrote:
> 
>>> Yet, here is an example of where we have effectively added a null pointer exception. > At the very least, this should be eliminated on Linux
>>> and just use the signal handling null pointer error mechanism!
>>
>> You're a few years late, as pretty much nobody agreed with me that the operating system handling of it was plenty.
> 
> I think you misunderstand, we have etc.linux.memoryerror that can actually throw an error on a null pointer using the signal handler.

Windows creates a exception, too, on null seg faults.


> I have a suggestion: eliminate this feature, and add a -npe switch to the compiler that errors on any null pointer usage. Asserts will be sprinkled in everywhere, but may be useful to someone debugging a nasty null pointer segfault somewhere.

It's just redundant to add these.

On Monday, July 10, 2017 11:26:43 AM MDT Walter Bright via Digitalmars-d wrote:
> On 7/9/2017 6:37 PM, Jonathan M Davis via Digitalmars-d wrote:
> > For some assertions, having more than a file and line number is nice (e.g. if you have messages for your pre-condition assertions, then you can make it so that when it fails, the programmer doesn't even need to look at the source code),
>
> He always needs to look at the source code. Asserts are for BUGS, and to fix bugs, you gotta look at the code. If asserts are being used as a substitute for input/environment errors, they're misused.
>
> In 40 years of asserts, I can't think of ever not visiting the file/line where it failed as the first thing I did to debug it.

Yes, assertions are used for catching bugs, but when they're used for DbC, the bug is in the caller, and if it's clear from the message that the caller violated a pre-condition as well as what that pre-condition was, then there really is no need for the programmer to look at the source code of the function being called. They just need to look at their code that's calling it and figure out how they screwed up and passed a bad value. Essentially, with pre-conditions, it's really the caller's code that's being tested, not the code where the assertion itself is.

Assertions which test the code that they're actually in, on the other hand, pretty much always require that you look at that code to figure out what's going on.

- Jonathan M Davis