https://issues.dlang.org/show_bug.cgi?id=18115

Basile B. <b2.temp@gmx.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |CTFE

--

https://issues.dlang.org/show_bug.cgi?id=18115

--- Comment #1 from Basile B. <b2.temp@gmx.com> ---
The '&&' RHS  shouldn't be evaluated even in this simplified test case:

```
int test()
{
    if (test.stringof.length >= 6 &&
        test.stringof[$-7..$] == "1234567") {}
    return 0;
}

enum a = test();
```

When trying to build commit by commit i've found that https://github.com/dlang/dmd/commit/9a57a965647ca7eef4c3149158b9256edb543f8a is the culprit of the regression.

--

https://issues.dlang.org/show_bug.cgi?id=18115

Mike Franklin <slavo5150@yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |slavo5150@yahoo.com

--- Comment #2 from Mike Franklin <slavo5150@yahoo.com> ---
I bisected with digger and found the culprit to be https://github.com/dlang/dmd/pull/7225

--

https://issues.dlang.org/show_bug.cgi?id=18115

Rainer Schuetze <r.sagitario@gmx.de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |r.sagitario@gmx.de

--- Comment #3 from Rainer Schuetze <r.sagitario@gmx.de> ---
The reduced test case fails for older versions, too, but that's because you are substracting 7 there instead of 6.

--

https://issues.dlang.org/show_bug.cgi?id=18115

--- Comment #4 from Rainer Schuetze <r.sagitario@gmx.de> ---
if you change the test to

int test()
{
    if (test.stringof.length < 7)
        return 0;
    return test.stringof[$-7..$] == "1234567";
}

enum a = test();


it fails for older versions, too. That's caused by the semantic analysis already trying to optimize expressions. I guess it must not do this in case of errors but defer it to the runtime error.

--

https://issues.dlang.org/show_bug.cgi?id=18115

Walter Bright <bugzilla@digitalmars.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bugzilla@digitalmars.com

--- Comment #5 from Walter Bright <bugzilla@digitalmars.com> ---
Rainer, not sure what you're saying. Is it invalid code in the test case, or a compiler problem?

--

https://issues.dlang.org/show_bug.cgi?id=18115

--- Comment #6 from Basile B. <b2.temp@gmx.com> ---
I've failed when trying to reduce the issue. Take the first one as test case.

```
module test;

class A23456{}

string foo()
{
    string result;
    mixin("alias m = " ~ __MODULE__ ~ ";");
    foreach (member; __traits(allMembers, m))
    {
        if (member.length > 5 && member[$-6..$] == "A23456")
            result ~= member ~ " ";
    }
    return result;
}

enum e = foo();
```

--

https://issues.dlang.org/show_bug.cgi?id=18115

--- Comment #7 from Rainer Schuetze <r.sagitario@gmx.de> ---
> Rainer, not sure what you're saying. Is it invalid code in the test case, or a compiler problem?

I'm not 100% sure. At runtime the code will always produce a RangeError, but the check before it causes it to never be executed. Should it still be a compile error?

Allowing the condition could make writing generic code simpler, but might also trigger the "unreachable code" warning later.

To change it, dmd.constfold must not produce errors (unconditionally), but keep the expressions that *might* cause an error at runtime or CTFE. Array indexing and divide by zero are probably affected, too.

--

https://issues.dlang.org/show_bug.cgi?id=18115

--- Comment #8 from Basile B. <b2.temp@gmx.com> ---
(In reply to Rainer Schuetze from comment #7)
> > Rainer, not sure what you're saying. Is it invalid code in the test case, or a compiler problem?
> 
> I'm not 100% sure. At runtime the code will always produce a RangeError, but the check before it causes it to never be executed. Should it still be a compile error?
> 
> Allowing the condition could make writing generic code simpler, but might also trigger the "unreachable code" warning later.
> 
> To change it, dmd.constfold must not produce errors (unconditionally), but keep the expressions that *might* cause an error at runtime or CTFE. Array indexing and divide by zero are probably affected, too.

Sorry, did you miss my previous answer ? Use the code from the first message. The regression is obvious.

--

https://issues.dlang.org/show_bug.cgi?id=18115

--- Comment #9 from Rainer Schuetze <r.sagitario@gmx.de> ---
> Sorry, did you miss my previous answer ? Use the code from the first message. The regression is obvious.

I noticed and it is consistent with what I tried to say before: the reduced
test case fails because it checks "test()" for length 6, but then tries to
access "test()"[$-7].

I think the problem actually existed before (as in my variation), but is now also visible in array comparisons a bit earlier.

--