www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 6094] New: && doesn't shortcut properly with CTFE

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

           Summary: && doesn't shortcut properly with CTFE
           Product: D
           Version: unspecified
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: DMD
        AssignedTo: nobody puremagic.com
        ReportedBy: jmdavisProg gmx.com


--- Comment #0 from Jonathan M Davis <jmdavisProg gmx.com> 2011-06-02 21:18:02
PDT ---
This

bool func(alias pred = "a == b")(dchar a, dchar b)
{
    enum defaultPred = is(typeof(pred) : string) && pred == "a == b";

    return defaultPred;
}

void main()
{
    func('a', 'b');
    func!((dchar a, dchar b){return false;})('a', 'b');
}


fails to compile, giving this error:

q.d(3): Error: incompatible types for ((__dgliteral1) == ("a == b")): 'bool
delegate(dchar a, dchar b)' and 'string'
q.d(11): Error: template instance q.main.func!(delegate bool(dchar a, dchar b)
{
return false;
}
) error instantiating

It's obviously trying to evaluate pred == "a == b" in spite of the fact that
is(typeof(pred) : string) failed, which means that && isn't shortcutting like
it's supposed to.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jun 02 2011
next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=6094


Steven Schveighoffer <schveiguy yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |schveiguy yahoo.com


--- Comment #1 from Steven Schveighoffer <schveiguy yahoo.com> 2011-06-02
21:27:28 PDT ---
Hm... I'm not so sure this is a valid requirement.

&& shortcuts *running* the code if the first test fails, but it doesn't
shortcut *compiling* the code.  I think you need to use a static if in this
case.

For example, I'd expect this also to fail to compile:

void main()
{
   int x;

   if(is(typeof(x) == string) && x == "5") {}
}

But I'd expect this to work:

void main()
{
   int x;

   static if(is(typeof(x) == string))
     if(x == "5") {}
}

I would expect something like this to work in your example instead:

static if(is(typeof(pred) : string))
   enum defaultPred = (pred == "a == b");
else
   enum defaultPred = false;

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jun 02 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=6094


Jonathan M Davis <jmdavisProg gmx.com> changed:

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


--- Comment #2 from Jonathan M Davis <jmdavisProg gmx.com> 2011-06-02 21:33:39
PDT ---
Hmmm. It's essentially what std.string.icmp tries to do:

    enum isLessThan = is(pred : string) && pred == "a < b";

so, obviously someone else was thinking that it should work (though given this
behavior, it's obviously a bug in icmp). So, it wasn't my idea at all, but you
do have a good point about shortcutting running code rather than the
compilation.

Bleh. I'd like it to work, but I think that you're right.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jun 02 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=6094


Andrei Alexandrescu <andrei metalanguage.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
                 CC|                            |andrei metalanguage.com
         Resolution|INVALID                     |


--- Comment #3 from Andrei Alexandrescu <andrei metalanguage.com> 2011-06-02
22:36:59 PDT ---
Let's leave this in. Short circuit evaluation during compilation is sensible
and simplifies a lot of code.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jun 02 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=6094


Brad Roberts <braddr puremagic.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |braddr puremagic.com


--- Comment #4 from Brad Roberts <braddr puremagic.com> 2011-06-02 22:50:41 PDT
---
Unless I'm missing something, actually intermixing evaluation and semantic
analysis enough to pull this sort of thing off would be enormously intrusive.

Additionally, do we _really_ want compile time code execution semantics to be
that different from runtime execution semantics?  My gut says, "No way."

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jun 02 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=6094



--- Comment #5 from Steven Schveighoffer <schveiguy yahoo.com> 2011-06-03
06:29:02 PDT ---
The problem I see with allowing this is, it won't compile as non-CTFE, even
though normally CTFE-able functions can still be used during runtime.

For example, if you have this:

string foo()
{
   if(0 && "hello" == 1)
      return "impossible!";
   return "abc";
}

Then how does one mark this as "don't compile this in normal mode, only compile
this during ctfe".  I suppose you could do version(ctfe) around the function,
but I feel this is just as easy (and more accurate) to redo with a static if.

I really think this bug is invalid, but I won't change it in case I'm wrong :)

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jun 03 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=6094



--- Comment #6 from Jonathan M Davis <jmdavisProg gmx.com> 2011-06-03 10:24:52
PDT ---
Well, if it were allowed, I would expect to only work in places that _had_ to
be evaluated at compile-time - such as the value of enums or inside template
constraints. It would have to be stuff where it may sense to shortcut the
compilation. Inside of a function which may or may not be called with CTFE
definitely wouldn't qualify for that.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jun 03 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=6094


Don <clugdbug yahoo.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |clugdbug yahoo.com.au


--- Comment #7 from Don <clugdbug yahoo.com.au> 2011-06-06 01:45:43 PDT ---
(In reply to comment #4)

 Unless I'm missing something, actually intermixing evaluation and semantic
 analysis enough to pull this sort of thing off would be enormously intrusive.
 
 Additionally, do we _really_ want compile time code execution semantics to be
 that different from runtime execution semantics?  My gut says, "No way."


There are *no* existing instances where CTFE ever does semantic analysis.
CTFE happens far too late for that. In fact CTFE isn't involved at all, the
desired behaviour would happen in the constant folding step -- but it doesn't
do semantic analysis either.
This would seem to be a request to change the semantics of the && operator to
allow things like:

enum bool XXX = false && undefined;
to compile. As well as being complicated to implement, I really don't think
that would be a good idea. It would introduce a corner case:

bool YYY = false && undefined;  // doesn't compile, still won't compile?
const bool ZZZ = false && undefined; // I think this must be forbidden.

is(typeof(false && undefined)) presumably would still return false, even though
it does compile in the XXX case.

Would any of these compile?

bool foo(bool z) { return z; }
enum bool AAA = foo(false && undefined);
enum int BBB = (false && undefined) ? 7 : 3;
enum int CCC = false ? undefined : 6;
enum bool DDD = true || undefined;
enum bool BBB = !((true || undefined)) && undefined2;

The thing which is particularly difficult about implementing this, is that you
cannot run the semantic pass on the second branch of an && expression, until
you have run the optimizer pass on the first branch.

And what happens with this:
enum bool CCC = is(typeof(false && undefined));
Currently that returns false.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jun 06 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=6094


Don <clugdbug yahoo.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |enhancement


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


Rob Jacques <sandford jhu.edu> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |sandford jhu.edu
           Severity|enhancement                 |regression


--- Comment #8 from Rob Jacques <sandford jhu.edu> 2011-06-29 22:01:37 PDT ---
I'm pretty sure this is a regression between DMD 2.052 and DMD 2.053. I found
this 'regression' in template constraints:

if( isPointer!T && isPointer!(pointerTarget!T) )

the problem is that if T is a string, then pointerTarget!T can not compile.
This wouldn't be an issue if that meant the template constraint failed
gracefully, but instead it halts compilation.

Anyways, there is the question of whether or not shortcutting is the correct
behavior.

From a performance point of view, as someone who has spent time optimizing
templates for compile times, anything that can reduce DMD's memory-usage or
compile times is a good thing.

From a practical point of view, being able to guard statements without using a
static if is great for template constraints and other short templates.

From a consistently point of view CTFE is already shortcutting everything
inside a if(!__ctfe){} block. (and probably other if(false){} blocks as well).
And we will never be able give up shortcutting if(!__ctfe){} blocks.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jun 29 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=6094


Don <clugdbug yahoo.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|regression                  |enhancement


--- Comment #9 from Don <clugdbug yahoo.com.au> 2011-06-29 23:19:32 PDT ---
(In reply to comment #8)

 I'm pretty sure this is a regression between DMD 2.052 and DMD 2.053. I found
 this 'regression' in template constraints:
 
 if( isPointer!T && isPointer!(pointerTarget!T) )
 
 the problem is that if T is a string, then pointerTarget!T can not compile.
 This wouldn't be an issue if that meant the template constraint failed
 gracefully, but instead it halts compilation.


No, that's not a regression. && was never defined to work in that way.
It's a Phobos bug which has been exposed.


 Anyways, there is the question of whether or not shortcutting is the correct
 behavior.
 
 From a performance point of view, as someone who has spent time optimizing
 templates for compile times, anything that can reduce DMD's memory-usage or
 compile times is a good thing.

 From a practical point of view, being able to guard statements without using a
 static if is great for template constraints and other short templates.
 
 From a consistently point of view CTFE is already shortcutting everything
 inside a if(!__ctfe){} block. (and probably other if(false){} blocks as well).
 And we will never be able give up shortcutting if(!__ctfe){} blocks.


That is COMPLETELY irrelevant. It has nothing in common. To repeat what I said
earlier: the constant folding behaviour of && does *not* involve CTFE. In fact,
it's not even a change to the constant folding; it's a change to the semantic
pass of &&.

What this request is: Given X && Y, if X always evaluates to false, do not
perform _any_ semantic analysis on Y. No matter what garbage it is.
Likewise for X || Y; if X is true, don't semantically analyse Y.

So, instead of 
 1. semantic analysis X and Y;
 2. constant fold X&&Y;
it would become:
 1. semantic X;
 2. constfold X;
 3. if (X is true) return true;
 4. semantic Y;
 5. constfold X&&Y.

This is clearly a major enhancement request and not a regression.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jun 29 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=6094


Rob Jacques <sandford jhu.edu> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|enhancement                 |regression


--- Comment #10 from Rob Jacques <sandford jhu.edu> 2011-06-29 23:53:43 PDT ---
(In reply to comment #9)

 (In reply to comment #8)

 I'm pretty sure this is a regression between DMD 2.052 and DMD 2.053. I found
 this 'regression' in template constraints:
 
 if( isPointer!T && isPointer!(pointerTarget!T) )
 
 the problem is that if T is a string, then pointerTarget!T can not compile.
 This wouldn't be an issue if that meant the template constraint failed
 gracefully, but instead it halts compilation.


 
 No, that's not a regression. && was never defined to work in that way.
 It's a Phobos bug which has been exposed.


Don, this compiled prior to DMD 2.053 and was in my code, not Phobos. It _is_ a
change from existing behavior. (Whether that change is a bug fix or a
regression is debatable)


 Anyways, there is the question of whether or not shortcutting is the correct
 behavior.
 
 From a performance point of view, as someone who has spent time optimizing
 templates for compile times, anything that can reduce DMD's memory-usage or
 compile times is a good thing.

 From a practical point of view, being able to guard statements without using a
 static if is great for template constraints and other short templates.
 
 From a consistently point of view CTFE is already shortcutting everything
 inside a if(!__ctfe){} block. (and probably other if(false){} blocks as well).
 And we will never be able give up shortcutting if(!__ctfe){} blocks.


 
 That is COMPLETELY irrelevant. It has nothing in common. To repeat what I said
 earlier: the constant folding behaviour of && does *not* involve CTFE. In fact,
 it's not even a change to the constant folding; it's a change to the semantic
 pass of &&.
 
 What this request is: Given X && Y, if X always evaluates to false, do not
 perform _any_ semantic analysis on Y. No matter what garbage it is.
 Likewise for X || Y; if X is true, don't semantically analyse Y.
 
 So, instead of 
  1. semantic analysis X and Y;
  2. constant fold X&&Y;
 it would become:
  1. semantic X;
  2. constfold X;
  3. if (X is true) return true;
  4. semantic Y;
  5. constfold X&&Y.
 
 This is clearly a major enhancement request and not a regression.


Thank you for explaining the situation. I had thought that the change in
behavior was due to CTFE being applied in more places and replacing the
existing constant folding, etc. Given that this isn't related to CTFE, then
this is definitely a regression, as DMD 2.052 constfolded X before semantic
analysis of Y. (at least inside of template constraints and the like)

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jun 29 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=6094


Don <clugdbug yahoo.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|regression                  |enhancement


--- Comment #11 from Don <clugdbug yahoo.com.au> 2011-06-30 00:42:19 PDT ---
(In reply to comment #10)

 (In reply to comment #9)

 (In reply to comment #8)

 I'm pretty sure this is a regression between DMD 2.052 and DMD 2.053. I found
 this 'regression' in template constraints:
 
 if( isPointer!T && isPointer!(pointerTarget!T) )
 
 the problem is that if T is a string, then pointerTarget!T can not compile.
 This wouldn't be an issue if that meant the template constraint failed
 gracefully, but instead it halts compilation.


 
 No, that's not a regression. && was never defined to work in that way.
 It's a Phobos bug which has been exposed.


 
 Don, this compiled prior to DMD 2.053 and was in my code, not Phobos. It _is_ a
 change from existing behavior. (Whether that change is a bug fix or a
 regression is debatable)


OK, I've figured this out.
The change in behaviour was because of this commit:
3bba5ca9514121324769cd0f6d2537545481433d
which suppresses spurious _error messages.

What was happening with X && Y was that an error message was being generated
while evaluating the Y, but because error messages were suppressed, you didn't
see the error message. This is the important thing: it has ALWAYS generated an
error message.
Then, && gets constant folded. The constant folding assumes there are no
errors, but because X is false, it const folds to false without looking at Y.
(If it did look at Y, it would have crashed).
This was incorrect behaviour, but normally it didn't matter, because an error
message had been displayed already anyway.
And finally, the template constraint didn't do a sanity check to see if any
errors had occurred, it simply checked the result.

The net effect of this was that (false && _error) normally didn't compile, but
if it was inside a template constraint, it did compile!

So it was definitely an accepts-invalid bug that got fixed. Not a regression.
It wasn't supposed to do that, and there's nothing in the spec to suggest that
it should have behaved in that way.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jun 30 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=6094



--- Comment #12 from Don <clugdbug yahoo.com.au> 2011-07-21 02:21:25 PDT ---
One last little bit of complexity about this issue. The code for
IfStatement::semantic() in statement.c contains this comment:

    // If we can short-circuit evaluate the if statement, don't do the
    // semantic analysis of the skipped code.
    // This feature allows a limited form of conditional compilation.

If this were actually true, it'd be a strong argument for changing the
behaviour of &&. But I suspect this comment is obsolete.
In version 0.116 and earlier, the code below used to compile:

void main()
{
    if (0) anyoldgarbage();
}

But starting with 0.117, it was rejected. My feeling is that the comment should
be removed from the source, and this bug closed as WONTFIX.

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


Andrei Alexandrescu <andrei metalanguage.com> changed:

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


--- Comment #13 from Andrei Alexandrescu <andrei metalanguage.com> 2011-07-21
07:36:15 PDT ---
I guess we'll just close it. You're the doc, Don.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jul 21 2011