On 5/1/2015 11:09 AM, deadalnix wrote:
> neither you nor walter provide a good sum up of your discussion.

My last post here: https://github.com/D-Programming-Language/dmd/pull/2885

On Friday, 1 May 2015 at 18:45:54 UTC, Walter Bright wrote:
> On 5/1/2015 11:09 AM, deadalnix wrote:
>> neither you nor walter provide a good sum up of your discussion.
>
> My last post here: https://github.com/D-Programming-Language/dmd/pull/2885

So, the plan is to make this a warning ? If so that sound like the right way forward.

On 5/1/2015 12:06 PM, deadalnix wrote:
> On Friday, 1 May 2015 at 18:45:54 UTC, Walter Bright wrote:
>> On 5/1/2015 11:09 AM, deadalnix wrote:
>>> neither you nor walter provide a good sum up of your discussion.
>>
>> My last post here: https://github.com/D-Programming-Language/dmd/pull/2885
>
> So, the plan is to make this a warning ? If so that sound like the right way
> forward.

You must be looking at something else. What I said, and I quote:
-------------------
I'll try to summarize:
1.this should have been better specified to begin with
2.there are incorrect usages of  if (arr)  in the wild
3.there are correct usages of  if (arr)  in the wild

The case  if (arr) ...arr[0]...  will produce a runtime error, so I am not terribly concerned that is a disastrous problem.

What does greatly concern me is this change will break a LOT of long standing, correct D code. This is a long standing complaint about D, and has driven away a lot of users. There are also the users driven way because they download existing D code, and that code doesn't compile. They don't really care why it doesn't compile, just that it doesn't compile, and they don't look any further.

Is  if(arr)  a problem big enough to merit taking this rather large risk? I'm skeptical. I suspect this kind of change and check would be appropriate for a D linter, and not be part of the core language.
----------------------

On Friday, 1 May 2015 at 09:54:43 UTC, Walter Bright wrote:
> This really blows. And things like that isnan => isNaN really has GOT TO STOP. Just today I found it broke some older piece of code I had that's perfectly correct.

I don't know anything about that one, so if that's a recent one,
I missed it, but we've been rejecting name changes like that for
a while now. I think that it's been a couple of years since the
last time I saw one merged in. There might be an exception or two
that I don't know about, but we've pretty much stopped doing
precisely because it's an aesthetic thing but does not actually
catch bugs or anything like that.

That being said, something like disallowing if(arr) is completely
different. It's like disallowing implicit fallthrough in switch
statements. The change may or may not be worth making, but it's
_not_ for aesthetics. It's in order to catch and prevent bugs.
And for whatever reason, you never seem to understand that not
all breaking changes are equal. Some - particularly those that
catch or prevent bugs - are frequently welcomed, and it annoys
people quite a bit when such changes are refused. I know that Don
loves to complain about that. Changes like getting rid of
implicit fallthrough in switch statements are desirable, because
it catches and prevents bugs. And disallowing if(arr) is in the
same boat. Maybe it's not used incorrectly on a frequent enough
basis to be worth the code breakage. That's an open question. But
the change is targeted at bugs, not aesthetics, like isnan =>
isNAN is. So, the two cases are completely different.

Personally, I'd advise that no one ever use if(arr) and that they
should always do if(arr !is null) or if(!arr.empty) so that it's
clear what they intended and so that they don't accidentally
check for null when they intended to check for empty. So, I think
that we would very much be better off with if(arr) being illegal.
I don't think that it's the end of the world if we keep it, just
like it wouldn't have been the end of the world if we'd keep
implicit fallthrough for switch statements, but I do think that
it's worth making the change, because it catches and prevents
bugs.

The only problem I see with making the change has been a couple
of very vocal folks who have some code where they used if(arr)
correctly, and they don't want to change their code, which I
understand, and maybe that's enough that we won't make the change
(since it's Andrei and Cybershadow who are complaining), but most
of us consider if(arr) (or any case where an array is implicitly
converted to bool) too error-prone to use and would _want_ it
found and removed from our code bases. And it _is_ the sort of
thing that many newbies will screw up.

Regardless, whether we make the change with if(arr) or not, I
think that main point here is that not all breaking changes are
equal. Some are for aesthetic purposes (e.g. isnan => isNaN) and
don't really help much beyond having the library be more
consistent and easier to learn, whereas others actually find and
prevent bugs or allow us to implement some new idiom that we
think is important. I really don't think that the simple fact
that a change will break existing code in and of itself says
whether we should make the change. It makes it so that the change
has to provide _much_ greater value than would be the case for a
non-breaking change - the bar on such changes needs to be higher
- but sometimes, they're worth it, and it seems like you tend to
reject any breaking change even if everyone else involved thinks
that it's worth it.

We _should_ be rejecting aesthetic changes at this point, but
that doesn't mean that we should reject _all_ breaking changes.
Given your attitude on stuff like this, I'm honestly very
surprised that you ever let implicit fallthrough on switch
statements be made illegal.

- Jonathan M Davis

On Saturday, 2 May 2015 at 09:20:50 UTC, Jonathan M Davis wrote:
> The only problem I see with making the change has been a couple
> of very vocal folks
> [...]
> (since it's Andrei and Cybershadow who are complaining)

"You either die a hero, or live long enough to see yourself become the villain."

> Given your attitude on stuff like this, I'm honestly very
> surprised that you ever let implicit fallthrough on switch
> statements be made illegal.

That was 4 years ago (v2.054). For some perspective, that was the same version when @safe and @property were introduced.

On 4/29/15 11:14 PM, Iain Buclaw via Digitalmars-d wrote:
> On 30 April 2015 at 04:41, Steven Schveighoffer via Digitalmars-d
> <digitalmars-d@puremagic.com> wrote:
>> 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.
>>
>
> https://github.com/D-Programming-Language/dlang.org/pull/981#commitcomment-10918439
>
> The two mov's followed by an 'or' suspiciously look like (.ptr | .length) to me.

Huh. I guess you are right. I never thought to test that, but it does indeed mean that.

I suppose for all intents and purposes, arrays with nonzero length and null pointer are so rare it may as well just be testing if the pointer is non-null.

>
>>> 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.
>>
>
> Yes, it does (it emits the same assembly as arrays).

OK, but in this case I think dg.funcptr is what we want to test, the dg.ptr isn't really relevant. And I don't think you'd find a case where dg.funcptr is null but dg.ptr isn't.

>>> 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).
>>
>
> It's still annoying to have to special-case non-scalar types in
> boolean contexts.  The front-end should do this lowering. :-)

None of these things really affect me, I never use if(arr) or if(aa), because I'm too skeptical of what they convey, and have been burned by those in the past. And if(dg) doesn't seem to have issues with how it's implemented.

If it was in a code review, I'd reject such usages.

I personally don't think if(arr) or if(aa) should EVER compile with any kind of lowering. The meaning is too non-obvious.

-Steve

On Friday, 1 May 2015 at 19:45:18 UTC, Walter Bright wrote:
> I suspect this kind of change and check would be appropriate for a D linter, and not be part of the core language.

Yes it would fit nicely into a static analyzer, but DMD is currently the only semantic analyzer we have.

On 4/30/15 5:34 AM, Byron Heads wrote:
> On Wed, 29 Apr 2015 22:44:22 -0400, Steven Schveighoffer wrote:
>
>> On 4/29/15 8:35 PM, Martin Nowak wrote:
>>> 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.
>>
>> I wonder if it's possible to fix this, as it is, IMO, the only
>> legitimate drawback of this change. Could we make the following work?
>>
>> if((auto ary = func()).length)
>>
>> -Steve
>
>
>
> I would have though this would work:
>
> if(auto x = foo(), !x.empty)
>

That's because it means this:

if(auto x = (foo(), !x.empty))

which I think would be invalid since you are using x before assignment.

-Steve

Back with some data, this change indeed discovered one bug in SDC. This was not the first one I'm aware of the confusing behavior, and looking for it in PR, still we have at least one case left.

On 5/2/15 4:06 AM, Vladimir Panteleev wrote:
> On Saturday, 2 May 2015 at 09:20:50 UTC, Jonathan M Davis wrote:
>> The only problem I see with making the change has been a couple
>> of very vocal folks
>> [...]
>> (since it's Andrei and Cybershadow who are complaining)
>
> "You either die a hero, or live long enough to see yourself become the
> villain."
>
>> Given your attitude on stuff like this, I'm honestly very
>> surprised that you ever let implicit fallthrough on switch
>> statements be made illegal.
>
> That was 4 years ago (v2.054). For some perspective, that was the same
> version when @safe and @property were introduced.

One data point: dlang.org visits in March have increased 50 times between 2011 and 2015. (Visits are defined per http://www.webalizer.org/webalizer_help.html.) I'll have more data at my DConf talk. -- Andrei