www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 11080] New: assert(`string`) should be forbidden

reply d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=11080

           Summary: assert(`string`) should be forbidden
           Product: D
           Version: D2
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: enhancement
          Priority: P2
         Component: DMD
        AssignedTo: nobody puremagic.com
        ReportedBy: temtaime gmail.com



Subj.

assert(`something going wrong`);

Compiles and never fails.
Sometimes it makes a mistake.

IMO assert should be:
void assert(bool, string = ``);

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 20 2013
next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=11080


monarchdodra gmail.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |monarchdodra gmail.com



Not sure it's valid: What you are seeing is basically a dynamic array
implicitly decaying to its pointer / boolean in a conditional clause. Some
(myself included) judge this feature should be deprecated anyways.

For example, this would be a "legit" use of "assert(string)":
string s;
assert(s);

So, overall, I think you are seeing a "special" case of a more generic problem,
eg:

while ("hello") ...
if ("hello") ...
auto k = "hello" ? ... : ... ;

All of the above I judge ambiguous.

I couldn't find any relevant entries about this, but the "issue" *has* been
beaten to death on the forums:
http://forum.dlang.org/thread/bwgnbflygowctlisistg forum.dlang.org

Enjoy :)

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 21 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=11080


Maxim Fomin <maxim maxim-fomin.ru> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |maxim maxim-fomin.ru



---
I think banning assert("Any array literal"); makes sense as it is always true
and may hide programmer error of forgetting to place some condition before
string. Empty array literal is a special case - it is currently implemented as
pair null + 0 size.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 21 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=11080




D allows implicit convertion from an array to bool ? It's... strange.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 21 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=11080


bearophile_hugs eml.cc changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bearophile_hugs eml.cc





 D allows implicit convertion from an array to bool ? It's... strange.


Right, I think this bug report should be closed. The design bug to fix is to
refuse the implicit convertion from a dynamic array to bool (and perhaps there
is already a bug report open on that).

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 21 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=11080


Temtaime <temtaime gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |INVALID



Okay. Then it's my mistake.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 21 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=11080




The bug report is mine, you could vote it:

http://d.puremagic.com/issues/show_bug.cgi?id=4733

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 21 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=11080


bearophile_hugs eml.cc changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|INVALID                     |





 Okay. Then it's my mistake.


On the other hand Issue 4733 is opened since three years, so I don't know if
Walter & Andrei agree with it. If Issue 4733 gets refused then adding a warning
or error for assert("something") is a good idea. So I thin it's better to keep
this open until there's an official answer on Issue 4733.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 21 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=11080




---




 Okay. Then it's my mistake.


 
 On the other hand Issue 4733 is opened since three years, so I don't know if
 Walter & Andrei agree with it. If Issue 4733 gets refused then adding a warning
 or error for assert("something") is a good idea. So I thin it's better to keep
 this open until there's an official answer on Issue 4733.


This may be a separate issue. For example, the problem you pointed out can be
solved by rewriting array conditional evoluation to return length and not ptr
which still technically allows to write assert(""). In such case this isssue
still has some value.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 21 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=11080






 This may be a separate issue. For example, the problem you pointed out can be
 solved by rewriting array conditional evoluation to return length and not ptr
 which still technically allows to write assert(""). In such case this isssue
 still has some value.


I don't really such much value in banning string literals in asserts. For
starters, it is awfully specific. Second, I have trouble seeing why literals
get such a special treatment, when "assert(format("error"))" is just as
"wrong". It'd be creating new rules to catch an error that virtually never
happens anyways, and catches it un-reliably to boot.

Finally, a valid use case I can see would be a user wanting to check that an
empty string *actually does* implicitly evaluate to non null:
static assert ("", "Error! string to bool evaluation rules have changed!");

Chances are `assert("hello")` was wrong useage yes, but I think it hardly
warrants new language rules...

...rules you wouldn't be able to apply to normal functions. For example an
"enforce" that refuses array literals. Can't happen.

A good rule of thumb is that if a built-in can do it, so should a user-built.
This would not be the case for this new rule.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 21 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=11080


Jonathan M Davis <jmdavisProg gmx.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jmdavisProg gmx.com



PDT ---
To completely accurate, strings do not implicitly convert to bool. Rather, in
conditions, the compiler inserts cast(bool). So, in conditions, anything which
can be explicitly cast to bool appears to be implicitly cast (when in fact it's
explicitly cast), but outside of conditions, there is not such conversion. e.g.

bool b = "foo";

will fail to compile.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 21 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=11080




---




 This may be a separate issue. For example, the problem you pointed out can be
 solved by rewriting array conditional evoluation to return length and not ptr
 which still technically allows to write assert(""). In such case this isssue
 still has some value.


 
 I don't really such much value in banning string literals in asserts. For
 starters, it is awfully specific. Second, I have trouble seeing why literals
 get such a special treatment, when "assert(format("error"))" is just as
 "wrong". It'd be creating new rules to catch an error that virtually never
 happens anyways, and catches it un-reliably to boot.


Assert("string") is a bug. There should be no discussion here. format("error")
is not an array literal, so it is irrelevant.


 Finally, a valid use case I can see would be a user wanting to check that an
 empty string *actually does* implicitly evaluate to non null:
 static assert ("", "Error! string to bool evaluation rules have changed!");


Ideally this should be documented and not be a subject to change. Anyway, one
can test ptr and length properties.


 Chances are `assert("hello")` was wrong useage yes, but I think it hardly
 warrants new language rules...


Assert("hello") is an uncoditional bug -  no need to calculate any chances.


 ...rules you wouldn't be able to apply to normal functions. For example an
 "enforce" that refuses array literals. Can't happen.


Enforce is irrelevant.


 A good rule of thumb is that if a built-in can do it, so should a user-built.
 This would not be the case for this new rule.


Built-in can not, but user-built can. This does not get into a conflict with
"if a built-in can do it, so should a user-built". (Arguments based on rules of
thumb coming from nowhere are hard to value). Contra point - dmd rejects some
potentially broken code, for example 'l' suffix for integer literal and
assignment in condition evaluation - which even is not necessarily a bug.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 21 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=11080






 Assert("string") is a bug. There should be no discussion here.


Why is it a bug? That's the discussion we're having. An array literal that
evaluates to null *will* trigger it. A user can test it.

void main()
{
    enum string s1 = "string";
    enum string s2 = null;

    assert( s1);
    assert(!s2);

    assert( "");
    assert(!string.init);
}

These all seem like legit use cases to me.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 21 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=11080


Andrej Mitrovic <andrej.mitrovich gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |andrej.mitrovich gmail.com



11:08:02 PDT ---



 These all seem like legit use cases to me.


Banning assert("") just seems like a pointless special case, we should either
disallow implicit conversion to bool or drop the issue altogether. Adding a
special rule for assert is a bad idea, we already have enough special rules
like assert(0).

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 21 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=11080


bearophile_hugs eml.cc changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |RESOLVED
         Resolution|                            |INVALID






     enum string s1 = "string";
     enum string s2 = null;
 
     assert( s1);
     assert(!s2);
 
     assert( "");
     assert(!string.init);
 }
 
 These all seem like legit use cases to me.


Using the bang (!) forces a explicit boolean conversion, that's different from
implicit conversion from string to bool.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 21 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=11080




PDT ---

 Why is it a bug?


I think that the argument is that no one would ever want to assert than an
array literal is true, because it's a given that it is. And if it's never
something that programmers are going to want to do, and there's significant
risk in doing that instead of assert(cond, "msg"); then it should be banned in
order to avoid that particular mistake.

That being said, I don't think that it's worth adding a special case to the
compiler for this. We should avoid special casing stuff as much as we
reasonably can, and I don't think that this problem is anywhere near big enough
to merit one. I assume that Temtaime ran into this problem, because (s)he
reported it, but I have never run into it - either in my own code or in anyone
else's code - and I've never even heard of anyone running it before this. So,
while clearly it has caused a problem for at least one person, I seriously
question that it's much of a problem in general. Worst case, it sounds like the
sort of thing that should be solved by a lint-like tool.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 21 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=11080







 That being said, I don't think that it's worth adding a special case to the
 compiler for this.


There is a much better and more general solution, from Issue 4733 .

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 21 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=11080




---




 Assert("string") is a bug. There should be no discussion here.


 
 Why is it a bug? That's the discussion we're having. An array literal that
 evaluates to null *will* trigger it. A user can test it.
 
 void main()
 {
     enum string s1 = "string";
     enum string s2 = null;
 
     assert( s1);
     assert(!s2);
 
     assert( "");
     assert(!string.init);
 }
 
 These all seem like legit use cases to me.


No, there is difference between array type object and array literal expression.
Code like assert("Array literal") (note that this is not assert(s) where 's'
refers to a string) is always a bug because the expression is always true and
indicates that user actually wanted assert(some_condition, "array literal").
There is no reason to write such code. As I have pointed out above, even in
situations which are not surely error, dmd still aborts compilation. In this
regard discussed issue is clear because there is no doubt whether use case is a
bug or not.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 21 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=11080


Maxim Fomin <maxim maxim-fomin.ru> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|INVALID                     |



---


 Why is it a bug?


 
 I think that the argument is that no one would ever want to assert than an
 array literal is true, because it's a given that it is. And if it's never
 something that programmers are going to want to do, and there's significant
 risk in doing that instead of assert(cond, "msg"); then it should be banned in
 order to avoid that particular mistake.
 
 That being said, I don't think that it's worth adding a special case to the
 compiler for this. We should avoid special casing stuff as much as we
 reasonably can, and I don't think that this problem is anywhere near big enough
 to merit one. I assume that Temtaime ran into this problem, because (s)he
 reported it, but I have never run into it - either in my own code or in anyone
 else's code - and I've never even heard of anyone running it before this. So,
 while clearly it has caused a problem for at least one person, I seriously
 question that it's much of a problem in general. Worst case, it sounds like the
 sort of thing that should be solved by a lint-like tool.


I think this is not a big deal to add such 'special rule' as it clearly adds
value to the language - discussed case is always a bug and nothing should be
hurt by such special case. I reopen issue (by the way, marking it as invalid is
wrong - there is wontfix for purposes you wish).

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 21 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=11080




---



 

 That being said, I don't think that it's worth adding a special case to the
 compiler for this.


 
 There is a much better and more general solution, from Issue 4733 .





 

 That being said, I don't think that it's worth adding a special case to the
 compiler for this.


 
 There is a much better and more general solution, from Issue 4733 .


Thanks for raising the problem, but this one is a separate issue.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 21 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=11080







 Thanks for raising the problem, but this one is a separate issue.


It's a separate issue, but if you disallow dynamic arrays in boolean evaluation
contexts, then it also disallows code like assert("something going wrong") and
you don't need to add a special rule to D for such buggy case.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 21 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=11080




---



 

 Thanks for raising the problem, but this one is a separate issue.


 
 It's a separate issue, but if you disallow dynamic arrays in boolean evaluation
 contexts, then it also disallows code like assert("something going wrong") and
 you don't need to add a special rule to D for such buggy case.


Yes, but one of the way to fix the issue (which I consider as a better one) is
to cast array not to pointer but to length and still allow arrays in boolean
conditions.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 21 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=11080





 ...rules you wouldn't be able to apply to normal functions. For example an


"enforce" that refuses array literals. Can't happen.

Actually it's the other way around. Currently, assert is special.

void enforce(bool b, string msg = "xxx")
{
}

void main()
{
   enforce("zzz");
}


zunk.d(8): Error: function zunk.enforce (bool b, string msg = "xxx") is not
callable using argument types (string)

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 23 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=11080




04:21:26 PDT ---


 ...rules you wouldn't be able to apply to normal functions. For example an


 "enforce" that refuses array literals. Can't happen.
 
 Actually it's the other way around. Currently, assert is special.
 
 void enforce(bool b, string msg = "xxx")
 {
 }
 
 void main()
 {
    enforce("zzz");
 }
 
 
 zunk.d(8): Error: function zunk.enforce (bool b, string msg = "xxx") is not
 callable using argument types (string)


That's not the Phobos enforce. This is:

-----
import std.exception;

void main()
{
   enforce("zzz");  // works fine
}
-----

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 23 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=11080








 ...rules you wouldn't be able to apply to normal functions. For example an


 "enforce" that refuses array literals. Can't happen.
 
 Actually it's the other way around. Currently, assert is special.
 
 void enforce(bool b, string msg = "xxx")
 {
 }
 
 void main()
 {
    enforce("zzz");
 }
 
 
 zunk.d(8): Error: function zunk.enforce (bool b, string msg = "xxx") is not
 callable using argument types (string)


 
 That's not the Phobos enforce. This is:


I think his point was that enforce *could* be written to not accept string
literals, countering my earlier point that "assert" would have a special
"no-string-literals", which would not have been possible to implement with
enforce.

I don't think the example is relevant though, because this new enforce would
*also* turn down:
string s;
enforce(s);

Which assert would still support.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 23 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=11080










 Assert("string") is a bug. There should be no discussion here.


 
 Why is it a bug? That's the discussion we're having. An array literal that
 evaluates to null *will* trigger it. A user can test it.
 
 void main()
 {
     enum string s1 = "string";
     enum string s2 = null;
 
     assert( s1);
     assert(!s2);
 
     assert( "");
     assert(!string.init);
 }
 
 These all seem like legit use cases to me.


 
 No, there is difference between array type object and array literal expression.
 Code like assert("Array literal") (note that this is not assert(s) where 's'
 refers to a string) is always a bug because the expression is always true and
 indicates that user actually wanted assert(some_condition, "array literal").
 There is no reason to write such code. As I have pointed out above, even in
 situations which are not surely error, dmd still aborts compilation. In this
 regard discussed issue is clear because there is no doubt whether use case is a
 bug or not.


Fine.

But in that case, as a variation of 4733, we could simply ban *any* "implicit
call to explicit conversion to bool" for *all* array literals, regardless of
type and context? It would be a step in the right direction (IMO), be more
generic, and cover other "most probably wrong" use cases. EG:

if ("hello"){}
assert([1, 2, 3]);

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 23 2013
prev sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=11080




05:46:52 PDT ---


 I think his point was that enforce *could* be written to not accept string
 literals


Well yeah, so now that's yet another special case. Then you'll have a bunch of
Phobos functions working, others not working, and 3rd party libraries doing
their own thing.

I agree that assert("foo") is meaningless, but I think we should fix it by
disallowing implicit bool conversion from a string. Can you really tell at a
glance whether `if (getString())` checks for ".length != 0" or "!is null"? I
can't, and I always have to look up the specs to remind myself (not in my code,
I don't use this form of implicit bool check).

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 23 2013