In Python it is a normal idiom to test for collection emptiness just putting their name in a boolean evaluation context (this is an idom valid for all Python collections): arr = [1] assert arr But in D similar code causes troubles, this differt code shows an example: void main() { int[] arr = [1]; int* orig_ptr = arr.ptr; arr.length = 0; assert(!!arr); // this doesn't assert assert(arr.ptr == orig_ptr); // this doesn't assert } !!arr is true because while arr.length is zero, arr.ptr is not null. To avoid possible bugs (for example for programmers coming from Python language) I suggest to turn "if(arr)" into a syntax error, forcing to use "if(arr.length)" or "if(arr.ptr)" or similar things, and you avoid possible bugs. In the uncommon cases where you want to test both fields to be empty, you may use "if(arr.length || arr.ptr)" or "if(arr.length != arr.init)" or similar things. The semantics of "if(aa)" and "if(static_arr)", the first tests if the reference is null, the second if all items are empty.
See also issue 7539
I have created a thread about this issue in the main D newsgroup: http://forum.dlang.org/thread/bwgnbflygowctlisistg@forum.dlang.org It was well received by almost every one. But people are split between just forbidding "if(dyn_arr)" and turning it into an equivalent of "if(dyn_arr.length)". Both proposals have advantages and disadvantages. Both are a little breaking change, but turning something into an error allows you to find it quickly in most cases inside D code. (But D also has the compiles __trait, that does not give an error, it just silently returns false instead of true). On the other hand the behaviour change is not meant to happen all at once. It has to be a warning, then deprecation and then a change, along months and three DMD versions. So there is time to fix old D code. And even if the old D code doesn't get changed, I think it's uncommon for code to rely on dynamic array to be actually (null,null) instead of being of zero length. So the total amount of breakage even for a semantic change is probably small. Considering the presence of the compiles __trait, the idea of the small behaviour change is not so bad. In both cases the change path will start at the same way: warning, deprecation, and then an error or a behavour change). So for now I am asking for a warning for code like "if(dyn_arr)". Note that the syntax "if(dyn_arr !is null)" is still allowed. -------------------- A related issue is what to do with associative arrays: void main() { bool[int] aa; assert(!aa); aa = [1: true]; assert(aa); aa.remove(1); assert(aa); assert(!aa.length); }
I feel that there is an additional bug: "if(slice)" evaluates an empty slice as true (as does "!!slice"), but "cast(bool)slice" does not compile ("cannot cast slice to integral type bool"). So somehow, converting arrays to booleans isn't consistently supported, and when it compiles, the meaning is indeed unintuitive. I agree that if conversion to booleans is to be supported, then "non-empty" is the best meaning to go for.
See also issue 11080, that asks to disallow this bug-prone construct: assert("something going wrong");
Warning: https://github.com/D-Programming-Language/dmd/pull/2885
Commit pushed to master at https://github.com/D-Programming-Language/druntime https://github.com/D-Programming-Language/druntime/commit/e5ed1beb8ebf86314136875801c2d929ddc32b9c Merge pull request #674 from yebblies/issue4733 Remove cases where an array is used in a boolean context
(In reply to comment #5) > Warning: > > https://github.com/D-Programming-Language/dmd/pull/2885 The new warning is: warning("converting dynamic arrays to boolean is deprecated, instead use array.ptr"); For normal D programmers the right way to tell if an array (and eventually associative array) is empty is with the std.array.empty function, that tests the length. You don't want future normal D programmers to start using .ptr everywhere they want to test array emptiness. So perhaps a better warning is: "converting dynamic arrays to boolean is deprecated, instead use std.array.empty" On the other hand some programmers want really meant to use ".ptr". And some programmers want to test "arr.length || arr.ptr" and so on. So perhaps an alternative error message is: "converting dynamic arrays to boolean is deprecated"
(In reply to comment #7) > (In reply to comment #5) > > Warning: > > > > https://github.com/D-Programming-Language/dmd/pull/2885 > > The new warning is: > > warning("converting dynamic arrays to boolean is deprecated, instead use > array.ptr"); > > For normal D programmers the right way to tell if an array (and eventually > associative array) is empty is with the std.array.empty function, that tests > the length. You don't want future normal D programmers to start using .ptr > everywhere they want to test array emptiness. > > So perhaps a better warning is: > > "converting dynamic arrays to boolean is deprecated, instead use > std.array.empty" > > On the other hand some programmers want really meant to use ".ptr". And some > programmers want to test "arr.length || arr.ptr" and so on. > > So perhaps an alternative error message is: > > "converting dynamic arrays to boolean is deprecated" Or simply: use arr.length != 0
How about: "converting dynamic arrays to boolean is deprecated, instead use std.array.empty or array.ptr"
(In reply to comment #8) > Or simply: use arr.length != 0 That is not the same as `if (arr)`, and I don't want this to make people accidentally break their code.
(In reply to comment #11) > (In reply to comment #8) > > Or simply: use arr.length != 0 > > That is not the same as `if (arr)`, and I don't want this to make people > accidentally break their code. No need to talk to me twice. Then !arr.length. That is the same as std.array.empty and it saves you the import.
(In reply to comment #12) > (In reply to comment #11) > > (In reply to comment #8) > > > Or simply: use arr.length != 0 > > > > That is not the same as `if (arr)`, and I don't want this to make people > > accidentally break their code. > > No need to talk to me twice. Then !arr.length. That is the same as > std.array.empty and it saves you the import. `if (!arr.length) is the same as `if (array.length != 0), and neither are the same as `if (arr)`, which expands to `if (arr.ptr)`.
(In reply to comment #13) > (In reply to comment #12) > > (In reply to comment #11) > > > (In reply to comment #8) > > > > Or simply: use arr.length != 0 > > > > > > That is not the same as `if (arr)`, and I don't want this to make people > > > accidentally break their code. > > > > No need to talk to me twice. Then !arr.length. That is the same as > > std.array.empty and it saves you the import. > > `if (!arr.length) is the same as `if (array.length != 0), and neither are the > same as `if (arr)`, which expands to `if (arr.ptr)`. I never said that it is the same. But it's the same as std.array.empty and saves you the import. That's all. https://github.com/D-Programming-Language/phobos/blob/master/std/array.d#L41 I would prefer that the warning consider arr.length != 0 more than std.array.empty. Nothing else. Hope you understand me now.
Commit pushed to master at https://github.com/D-Programming-Language/phobos https://github.com/D-Programming-Language/phobos/commit/218ad0bf43299bea71b98a9f2bf8e05612ee23cd Merge pull request #1727 from yebblies/issue4733 Remove cases where an array is used in a boolean context
(In reply to comment #14) > I would prefer that the warning consider arr.length != 0 more than > std.array.empty. Nothing else. Hope you understand me now. Suggesting people to use length is not a good idea. For that semantics there is empty that is safer and more clean, and it's the preferred idiom in D code. (I have an enhancement request to extend the "empty" to work on associative arrays too, Issue 6409 ). The problem yebblies is explaining is that the code going to be disallowed is "if(arr)" that doesn't have the same semantics as "if(!arr.empty)", despite the programmer often meant that semantic. "if(arr)" is semantically unclean, that's why I have written this tiny breaking enhancement request to disallow it. And it's right such semantic confusion that makes it harder to write a good warning message for this. "converting dynamic arrays to boolean is deprecated, instead use std.array.empty or arr.ptr" That message confuses a bit the "std.array" module with the "array" as the data to work on. So an alternative is to write the name of the real array used in the code: int[] foo; if (foo) {} ==> "converting dynamic arrays to boolean is deprecated, instead use std.array.empty or foo.ptr" I think this is the best simple warning I can think of for now.
(In reply to comment #16) > (In reply to comment #14) > > > I would prefer that the warning consider arr.length != 0 more than > > std.array.empty. Nothing else. Hope you understand me now. > > Suggesting people to use length is not a good idea. For that semantics there is > empty that is safer and more clean, and it's the preferred idiom in D code. Not in any D code I've written or I've seen so far. But thanks for explanation. I use already arr.length != 0 anyway. :) > (I have an enhancement request to extend the "empty" to work on associative > arrays too, Issue 6409 ).
(In reply to comment #10) > (In reply to comment #8) > > Or simply: use arr.length != 0 > > That is not the same as `if (arr)`, and I don't want this to make people > accidentally break their code. The difference between !!arr.ptr and !!arr.length is always a bug, because accessing an array with non-null ptr but zero length is a range error.
1. making this change will break existing code 2. forcing people to use if(arr.ptr) instead is just ugly
(In reply to comment #19) > 1. making this change will break existing code Yes, it's one of the small breaking changes, this one I have asked since 2010-08-26. There are about ten more tiny breaking changes I am asking since lot of time in Bugzilla. Do we want to close them all as WONTFIX? Having similar warts in D is not good. > 2. forcing people to use if(arr.ptr) instead is just ugly It's not something you do often. I don't think I have 1 case in my code where I need to do this. In 99% of the D code using ".empty" or even ".length == 0" is more clear and less dangerous (because it's more clear). The problems with finding a good error message show the old situation is not clear. The code being a tiny bit ugly could be an advantage if it helps show unusual (low-level) qualities of the code. Even using cast(int) is a little ugly compared to (cast) but that was done for some practical reasons. Using ".ptr" shows it's special code, while using empty or length shows it's normal D user code fit for @safe functions too. The idea that instances int[] are "pointers" is not a good idea. They are at best fat pointers, and conflating the idea of pointer with the idea of dynamic array was a design mistake that we are slowly fixing with tiny/small breaking changes. This ER is another step in that direction.
(In reply to comment #20) > Yes, it's one of the small breaking changes, ... As usual, sorry for the little aggressive tone of some of my answers, Walter.
> It's not something you do often. I don't think I have 1 case in my code where I > need to do this. I have few usages of array.ptr in my D code: for variable length structs that contain a zero-length array, for interfacing with C code, etc, but none of them come from "if(array)".
(In reply to comment #19) > 1. making this change will break existing code Unclear code *should* be broken. > 2. forcing people to use if(arr.ptr) instead is just ugly Yes, they should use if(!arr.empty) or if(arr.length>0) instead.
Let me restate the obvious. The following code has a bug. if (arr) writeln(arr[0]); I don't understand why this causes so much discussion. > making this change will break existing code Making this change will find broken code. Please come up with a valid example.
Commit pushed to master at https://github.com/D-Programming-Language/phobos https://github.com/D-Programming-Language/phobos/commit/6a8d98dd296448b3e840660b30f98db79b1b4f89 Merge pull request #1777 from yebblies/issue4733again More implicit array to bool
Commit pushed to master at https://github.com/D-Programming-Language/druntime https://github.com/D-Programming-Language/druntime/commit/3f50d37b2bed43a82fda3c5fee47b63eaa8765c1 missing executeArgs for benchmarks - avoid implicit array to boolean conversion (Issue 4733)
Commits pushed to master at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/727233c9c5c1d81d13527f74b0a640f546f28a6c Fix Issue 4733 - Possible bugs caused by dynamic arrays in boolean evaluation context Turn it into a warning https://github.com/D-Programming-Language/dmd/commit/b9177ab9322f8fac7f20f279a5517eda26734ef9 Merge pull request #2885 from yebblies/issue4733 Issue 4733 - Possible bugs caused by dynamic arrays in boolean evaluation context
Commits pushed to master at https://github.com/D-Programming-Language/dlang.org https://github.com/D-Programming-Language/dlang.org/commit/baa98b725ac75356a29405c164743f7cea7d502d Add changelog entry for issue4733 https://github.com/D-Programming-Language/dlang.org/commit/5432fb77472a0f11078fb407a3ccc256189ec4db Merge pull request #964 from yebblies/changelog4733 Add changelog entry for issue4733
Commits pushed to master at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/72e8bfd2ac371d9a2acc34d7313882a5c4547ce0 Revert "Fix Issue 4733 - Possible bugs caused by dynamic arrays in boolean evaluation context" This reverts commit 727233c9c5c1d81d13527f74b0a640f546f28a6c. https://github.com/D-Programming-Language/dmd/commit/ef0bb27a609ffa9102a7bac713e4f418f16e92c1 Merge pull request #4623 from andralex/fix-array-to-bool-warning Revert "Fix Issue 4733 - Possible bugs caused by dynamic arrays in boole...
Commits pushed to stable at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/727233c9c5c1d81d13527f74b0a640f546f28a6c Fix Issue 4733 - Possible bugs caused by dynamic arrays in boolean evaluation context https://github.com/D-Programming-Language/dmd/commit/b9177ab9322f8fac7f20f279a5517eda26734ef9 Merge pull request #2885 from yebblies/issue4733 https://github.com/D-Programming-Language/dmd/commit/72e8bfd2ac371d9a2acc34d7313882a5c4547ce0 Revert "Fix Issue 4733 - Possible bugs caused by dynamic arrays in boolean evaluation context" https://github.com/D-Programming-Language/dmd/commit/ef0bb27a609ffa9102a7bac713e4f418f16e92c1 Merge pull request #4623 from andralex/fix-array-to-bool-warning
Commits pushed to stable at https://github.com/D-Programming-Language/dlang.org https://github.com/D-Programming-Language/dlang.org/commit/baa98b725ac75356a29405c164743f7cea7d502d Add changelog entry for issue4733 https://github.com/D-Programming-Language/dlang.org/commit/5432fb77472a0f11078fb407a3ccc256189ec4db Merge pull request #964 from yebblies/changelog4733
The compiler change was reverted.
I think it's pretty safe to say, this will not be fixed. I can't see Walter/Andrei going back on this revert. Better to not have open bugs that will never be resolved, even if we don't agree with the outcome.
*** Issue 7539 has been marked as a duplicate of this issue. ***
Commits pushed to dmd-cxx at https://github.com/dlang/dmd https://github.com/dlang/dmd/commit/727233c9c5c1d81d13527f74b0a640f546f28a6c Fix Issue 4733 - Possible bugs caused by dynamic arrays in boolean evaluation context https://github.com/dlang/dmd/commit/b9177ab9322f8fac7f20f279a5517eda26734ef9 Merge pull request #2885 from yebblies/issue4733 https://github.com/dlang/dmd/commit/72e8bfd2ac371d9a2acc34d7313882a5c4547ce0 Revert "Fix Issue 4733 - Possible bugs caused by dynamic arrays in boolean evaluation context" https://github.com/dlang/dmd/commit/ef0bb27a609ffa9102a7bac713e4f418f16e92c1 Merge pull request #4623 from andralex/fix-array-to-bool-warning
For the record, this post described why the revert happened: https://forum.dlang.org/post/mr540j$1533$1@digitalmars.com "The main reason why it caused issues is this nice idiom: if(auto arr = someFunction()) { // use arr } This would HAVE to be split out to two statements, and the arr variable would be scoped outside of the if statement." I attempted to make it obsolete: https://github.com/dlang/dmd/pull/15413 But that was rejected. I still think it would be good to do for editions. If that is still a no-go then it seems we need something like: if ((auto arr = expr).ptr) However, would `arr` then be declared in the `else` branch? If so that is still not a replacement for the feature. Another option would be to allow `if (auto arr = expr)` but allow no other uses of an array as a boolean.
AIU some believe it's a feature, not a bug? Then it becomes a style rule. Editions are only supposed for consensus.
Isn't there a consensus that the feature is bug prone (at the very least for new users)?
I would vote for `if(arr)` to be true only if length > 0, instead of making it illegal. But some people do distinguish between null arrays and empty arrays, and this would be a breaking change for that code. > I still think it would be good to do for editions. Yes, please! If editions ever becomes a thing, this should be considered.
> If that is still a no-go then it seems we need something like: > > if ((auto arr = expr).ptr) In C++17, you can write it like this: if (auto arr = expr; arr.ptr)
kdevel adds detail that `if (arr.ptr)` and `if (arr)` are not the same! The difference is significant for typeid(T).initializer, which has a null pointer but non-zero length: https://forum.dlang.org/post/yvhwvufbegsvupvvsdbe@forum.dlang.org Note that `if (arr !is null)` is equivalent to `if (arr)`. Those check both the .ptr and .length fields (i.e. `is null` means exactly the same as `is []`).
(In reply to Nick Treleaven from comment #43) > Note that `if (arr !is null)` is equivalent to `if (arr)`. Those check both > the .ptr and .length fields (i.e. `is null` means exactly the same as `is > []`). To be 100% explicit: both `if (arr !is null)` and `if (arr)` are equivalent to `if (a.ptr || a.length)`.