Steven Schveighoffer:

> FYI, Andrei and Walter are reversing this change barring any new evidence it's helpful to people. Please speak up if you disagree.

There's no more evidence. It's an improvement, for people coming from Python. The current semantics is not meaningful. One of the points of D over C++ was to fix irrationally designed parts, like this small problem. Many small design mistakes like this one create C++ we know and hate. We should fix such small problems quickly and look forward, instead of debating forever and reverting every small step forward like this. My presence around here is becoming useless.

Bye,
bearophile

On 29 April 2015 at 06:38, Steven Schveighoffer via Digitalmars-d <digitalmars-d@puremagic.com> wrote:
> FYI, Andrei and Walter are reversing this change barring any new evidence it's helpful to people. Please speak up if you disagree.
>
> https://github.com/D-Programming-Language/dmd/pull/2885#issuecomment-97299912
>
> -Steve

I disagree, and I would extend my thoughts to say that I don't think that the change is enough.  We should also be warning on delegates and associative arrays too!

if (arr)  // Implicitly converted to (arr.ptr | arr.length) != 0
if (dg)  // Implicitly converted to (dg.ptr | dg.funcptr) != 0
if (aa)  // Implicitly converted to (aa.ptr != null)

Iain

On Wednesday, 29 April 2015 at 04:38:12 UTC, Steven Schveighoffer wrote:
> FYI, Andrei and Walter are reversing this change barring any new evidence it's helpful to people. Please speak up if you disagree.
>
> https://github.com/D-Programming-Language/dmd/pull/2885#issuecomment-97299912

I disagree, too. For me, the value of this change is not in detecting bugs, but in cleaning up a design mistake. It therefore doesn't matter whether there is evidence it helps avoiding bugs.

However, I did find several ones in my own code and in vibe.d:

https://github.com/rejectedsoftware/vibe.d/commit/e9e66f4e726db64d15e078dc472606b57783728a#diff-a0c0675933703d01a5d6ad8ebfc097abL79
https://github.com/D-Programming-Language/dub/pull/556/files

and similar ones with assert().

On Wednesday, 29 April 2015 at 11:07:52 UTC, Marc Schütz wrote:
://github.com/rejectedsoftware/vibe.d/commit/e9e66f4e726db64d15e078dc472606b57783728a#diff-a0c0675933703d01a5d6ad8ebfc097abL79

Are you talking about the last diff in particular? Because I would argue that that one is a poor design decision in Vibe (overloaded functions' return values should have the same semantics). What if one overload returned "bool" for success/failure and another a "size_t" for number of characters written to the error message buffer?

> https://github.com/D-Programming-Language/dub/pull/556/files

Forbidding arrays for assert and enforce conditions sounds like a good idea.

On Wednesday, 29 April 2015 at 11:28:02 UTC, Vladimir Panteleev wrote:
> On Wednesday, 29 April 2015 at 11:07:52 UTC, Marc Schütz wrote:
> ://github.com/rejectedsoftware/vibe.d/commit/e9e66f4e726db64d15e078dc472606b57783728a#diff-a0c0675933703d01a5d6ad8ebfc097abL79
>
> Are you talking about the last diff in particular? Because I would argue that that one is a poor design decision in Vibe (overloaded functions' return values should have the same semantics). What if one overload returned "bool" for success/failure and another a "size_t" for number of characters written to the error message buffer?

You're probably right, it's not the best design. But still, the original version compiled fine because that string was implicitly convertible to bool, which the DMD change caught.

On 4/29/15 7:05 AM, "Marc =?UTF-8?B?U2Now7x0eiI=?= <schuetzm@gmx.net>" wrote:
> On Wednesday, 29 April 2015 at 11:28:02 UTC, Vladimir Panteleev wrote:
>> On Wednesday, 29 April 2015 at 11:07:52 UTC, Marc Schütz wrote:
>> ://github.com/rejectedsoftware/vibe.d/commit/e9e66f4e726db64d15e078dc472606b57783728a#diff-a0c0675933703d01a5d6ad8ebfc097abL79
>>
>>
>> Are you talking about the last diff in particular? Because I would
>> argue that that one is a poor design decision in Vibe (overloaded
>> functions' return values should have the same semantics). What if one
>> overload returned "bool" for success/failure and another a "size_t"
>> for number of characters written to the error message buffer?
>
> You're probably right, it's not the best design. But still, the original
> version compiled fine because that string was implicitly convertible to
> bool, which the DMD change caught.

I have no doubt the change can find certain errors. Problem is false positives. FWIW these are the changes I had to operate on std.allocator to make it work with the new compiler. One per 194 lines on average, all false positives:

https://github.com/andralex/phobos/commit/4c14bf48fb5754782aec2380d41529eba3f2357b


Andrei

On Wednesday, 29 April 2015 at 04:38:12 UTC, Steven Schveighoffer wrote:
> FYI, Andrei and Walter are reversing this change barring any new evidence it's helpful to people. Please speak up if you disagree.
>
> https://github.com/D-Programming-Language/dmd/pull/2885#issuecomment-97299912
>
> -Steve

I don't have precise number to show, but I've been bitten by if (arr) doing null check rather than length check more than once.

The most problematic part of it is that it is frequent to have empty slice having a null pointer, so the code is likely to pass testing, while not doing what you expect it to do, and will break later in some bizarre way.

More generally, it is a identity vs equality problem. Slices have a semantic where equality is not identity ( == is different than is ) so the current behavior is inconsistent.

On Wednesday, 29 April 2015 at 17:35:58 UTC, Andrei Alexandrescu wrote:
> I have no doubt the change can find certain errors. Problem is false positives. FWIW these are the changes I had to operate on std.allocator to make it work with the new compiler. One per 194 lines on average, all false positives:

Yeah, but I think that it's safe to say that std.allocator is not the normal case, since it's operating so heavily on arrays (and non-GC allocated no less), and it's caring about their pointers rather than what they point to, whereas most D array code doesn't care about the ptr value of the array at all. In general, at most, it'll care about whether the array is empty or not. So, I would expect that for most programs, they won't see this warning much - be it a false positive or not - and if they do see it, the odds are pretty high that it's not a false positive. Yes, there will be code out there which uses if(arr) correctly, and not being able to do if(auto ptr = arr) will be annoying for some folks, but given how null and empty are so frequently conflated with arrays in D, I very much doubt that much code is going to be using if(arr) and want it to be equivalent to if(arr !is null).

Granted, we can't know for sure how common if(arr) is used in the wild - be it correctly or incorrectly - but std.allocator is definitely not your average code, and the consensus seems to be that if(arr) is error-prone even if it's occasionally useful.

- Jonathan M Davis

On 04/29/2015 09:15 PM, Jonathan M Davis wrote:
> Yeah, but I think that it's safe to say that std.allocator is not the normal case

std.allocator really isn't a general example.

I'm exclusively using if (ary.length) or if (!ary.empty) in my code to
avoid the problem.
Occasionally I'm using if (auto ary = func()), despite the fact that the
semantics are wrong, but it's nice and short and works as long a func
always returns null instead of empty slices.
But it's very fragile, hard to spot during debugging, and I already
spent too many hours on that.

So I'd expect any code analyzer to fault such usage and any D book to teach people not to use arrays as boolean, at which point we'd be better off to slowly remove it from the language.

On 4/29/15 4:29 AM, Iain Buclaw via Digitalmars-d wrote:
> On 29 April 2015 at 06:38, Steven Schveighoffer via Digitalmars-d
> <digitalmars-d@puremagic.com> wrote:
>> FYI, Andrei and Walter are reversing this change barring any new evidence
>> it's helpful to people. Please speak up if you disagree.
>>
>> https://github.com/D-Programming-Language/dmd/pull/2885#issuecomment-97299912
>>
>
> I disagree, and I would extend my thoughts to say that I don't think
> that the change is enough.  We should also be warning on delegates and
> associative arrays too!
>
> if (arr)  // Implicitly converted to (arr.ptr | arr.length) != 0

if(arr) simply means if(arr.ptr) != 0. Length does not come into play.

> if (dg)  // Implicitly converted to (dg.ptr | dg.funcptr) != 0

I don't know if I've ever expected that, you sure that's true? I would actually expect if(dg) to be equivalent to if(dg.funcptr). I have no idea how a dg would have a null pointer but valid funcptr.

> if (aa)  // Implicitly converted to (aa.ptr != null)

This is easily fixed when we fix aa to be a library type. We don't need compiler help to fix this issue (we do need compiler help to remove AA specialty code from the compiler, but once it's out, we can fix this without effort).

-Steve