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


Jonathan M Davis <jmdavisProg@gmx.com> changed:

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


--- Comment #10 from Jonathan M Davis <jmdavisProg@gmx.com> 2013-09-21 10:43:48 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: -------

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



--- Comment #11 from Maxim Fomin <maxim@maxim-fomin.ru> 2013-09-21 10:52:16 PDT ---
(In reply to comment #9)
> (In reply to comment #8)
> > 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: -------

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



--- Comment #12 from monarchdodra@gmail.com 2013-09-21 10:58:23 PDT ---
(In reply to comment #11)
> 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: -------

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


--- Comment #13 from Andrej Mitrovic <andrej.mitrovich@gmail.com> 2013-09-21 11:08:02 PDT ---
(In reply to comment #12)
> (In reply to comment #11)
> 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: -------

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


bearophile_hugs@eml.cc changed:

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


--- Comment #14 from bearophile_hugs@eml.cc 2013-09-21 11:27:21 PDT ---
(In reply to comment #12)

>     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: -------

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



--- Comment #15 from Jonathan M Davis <jmdavisProg@gmx.com> 2013-09-21 11:28:57 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: -------

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



--- Comment #16 from bearophile_hugs@eml.cc 2013-09-21 11:37:28 PDT ---
(In reply to comment #15)

> 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: -------

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



--- Comment #17 from Maxim Fomin <maxim@maxim-fomin.ru> 2013-09-21 14:29:19 PDT ---
(In reply to comment #12)
> (In reply to comment #11)
> > 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: -------

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                     |


--- Comment #18 from Maxim Fomin <maxim@maxim-fomin.ru> 2013-09-21 14:33:12 PDT ---
(In reply to comment #15)
> > 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: -------

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



--- Comment #19 from Maxim Fomin <maxim@maxim-fomin.ru> 2013-09-21 14:34:33 PDT ---
(In reply to comment #16)
> (In reply to comment #15)
> 
> > 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 .

(In reply to comment #16)
> (In reply to comment #15)
> 
> > 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: -------