January 11, 2017
On Wednesday, 11 January 2017 at 18:51:08 UTC, Andrei Alexandrescu wrote:
> On 1/11/17 4:25 PM, Stefan Koch wrote:
>> You should not need to special case ctfe inside the compiler for this.
>> Rather the template should have if (__ctfe) inside it if those are needed.
>> However I would advise against splitting code-paths, if it is not
>> _strictly_ necessary.
>
> That's what confuses me, it's the read of the temporary not the code inside the function. Would branching inside the function help with that? -- Andrei

Depending on how you choose optimize the certain cases.
You e.g. using sse2 instructions for copies of 128bit values,
It may become non-ctfeable.
Then you need a ctfeable branch in order for the function to still work at ctfe.
January 11, 2017
On 1/11/17 7:02 PM, Martin Nowak wrote:
> Don't really understand your question. What are the 2 problems you refer
> to?

The current implementation:

D _d_arraycopyT(S, D)(S from, D to, uint size)
{
    import core.stdc.string;
    (() @trusted => memcpy(cast(void*)to.ptr, from.ptr, to.length * size))();
    return to;
}

The current error:

ctfe.d(28): Error: 2 variable __r57 cannot be read at compile time
ctfe.d(28):        called from here: _d_arraycopyT(this.s[], __r57, 1u)

i.e. NOT in the use of cast, ptr, or memcpy. The error points to the call site.

The recommended implementation:

D _d_arraycopyT(S, D)(S from, D to, uint size)
{
    if (__ctfe)
        import core.stdc.string;
        (() @trusted => memcpy(cast(void*)to.ptr, from.ptr, to.length * size))();
    }
    else
    {
        foreach (i, ref e; from) to[i] = e;
    }
    return to;
}

It doesn't seem like the recommended implementation will make the error go away.

> The difference is fairly simple but huge:
>
> - C intrinsics
>   - AssignExp.semantic
>     - e2ir => call RTLSYM_SYM
>     - interpret => special handling
>
> - D lowering
>   - AssignExp.semantic lowered to CallExp of object._arrayCopy
>     - normal function call and no special CTFE handling

I'm not familiar with the code, so although yes these are different I don't know how they translate into what needs to be done to make this work.

>> Also, as an aside: the _d_arraycopyT should probably go like this:
>>
>> D _d_arraycopyT(S, D)(S from, D to) { ... }
>> You don't need size because it's from[0].sizeof. Correct?
>
> Just convert the assignment to a function call, the backend deals with
> optimizations et.al. Also this seems to be used not only for static arrays.

I'm saying the third parameter is entirely redundant in all cases. It's the sizeof the element.

> NB:
>   - leave aways the _d prefix it's only needed to namespace extern(C)
> functions with flat mangling
>   - prolly should be _arrayCopy(T)(in T[] from, T[] to) as
> AssignExp.semantic already takes care of conversions

Yah, once we're past the hurdle of getting this to basically work these are good touches.


Andrei

January 11, 2017
On Wednesday, 11 January 2017 at 18:59:29 UTC, Andrei Alexandrescu wrote:
>         foreach (i, ref e; from) to[i] = e;
I would guess foreach(i; auto ref e; from) to[i] = e;
is a little more compatible.
January 11, 2017
On 1/11/17 8:34 PM, Stefan Koch wrote:
> On Wednesday, 11 January 2017 at 18:59:29 UTC, Andrei Alexandrescu wrote:
>>         foreach (i, ref e; from) to[i] = e;
> I would guess foreach(i; auto ref e; from) to[i] = e;
> is a little more compatible.

Why? Aren't operands at this point built-in slices? -- Andrei
January 11, 2017
On Wednesday, 11 January 2017 at 22:14:18 UTC, Andrei Alexandrescu wrote:
> On 1/11/17 8:34 PM, Stefan Koch wrote:
>> On Wednesday, 11 January 2017 at 18:59:29 UTC, Andrei Alexandrescu wrote:
>>>         foreach (i, ref e; from) to[i] = e;
>> I would guess foreach(i; auto ref e; from) to[i] = e;
>> is a little more compatible.
>
> Why? Aren't operands at this point built-in slices? -- Andrei

it could still be that you are assigning from a const slice.
January 11, 2017
On 1/11/2017 10:51 AM, Andrei Alexandrescu wrote:
> That's what confuses me, it's the read of the temporary not the code inside the
> function. Would branching inside the function help with that? -- Andrei


The example is about 40 lines of code. The way to figure it out is to simplify the example.

For starters, replace:

  char[] mangle(T)(const(char)[] fqn, char[] dst = null)

with:

  char[] mangle(const(char)[] fqn)

Note that the template doesn't even use T or dst. Nor does it use unsignedToTempString.

Replace numDigits() with 1.

Rewrite the foreach loop to use primitives, then reduce the primitives (like removing popFront() which does nothing).

Never going to figure out what is wrong if the example is larded up with obfuscation and distraction. I bet it will shrink down to 3 lines that exhibit the same issue, at which point the problem will be obvious.
January 11, 2017
Take a look at the D test suite, such as:

    https://github.com/dlang/dmd/blob/master/test/runnable/test42.d

Each of these tests cases started out as some long complicated thing that got reduced to a very small self-contained test case.
January 12, 2017
On Wednesday, 11 January 2017 at 17:10:15 UTC, Andrei Alexandrescu wrote:
>> Calls for the old dmd<->C-API are very different from template functions
>> calls, e.g. take a look at how _xOpEquals is called.
>> https://github.com/dlang/dmd/blob/538a895157acdbbfc5869791f9504f7e86b4fdd0/src/clone.d#L496
>

Sorry, it's not the best example, b/c that also generates a function.
Added inline comments on https://github.com/somzzz/dmd/commit/8bccc49ba661567c523545650aad30c01fd25090 which isn't far off.

> Or perhaps that's why the error with reading the variable during compilation?

Not sure where that comes from, there are unrelated changes as well.

January 12, 2017
On Wednesday, 11 January 2017 at 23:44:29 UTC, Stefan Koch wrote:
> it could still be that you are assigning from a const slice.

In which case ref for the foreach parameter still works.
January 16, 2017
On Thursday, 12 January 2017 at 01:29:12 UTC, Walter Bright wrote:
> Take a look at the D test suite, such as:
>
>     https://github.com/dlang/dmd/blob/master/test/runnable/test42.d
>
> Each of these tests cases started out as some long complicated thing that got reduced to a very small self-contained test case.

Here is a smaller example:

ptrdiff_t f(char[] s) {
    // foreach(c; s) if (c == '.') return 0; // (0)fails with the same error message
    foreach(char c; s) if (c == '.') return 0; // (1)
    // foreach(dchar c; s) if (c == '.') return 0; // (2) different compile path, doesn't fail

    /*
    for (size_t i = 0; i < s.length; i++)
    {
        if (s[i] == '.') return i;
    }
    */
    return -1;
}

pragma(msg, f(['z', 'b', 'c', '.', 'd']));

-------------------------------------------------------

ctfe.d(5): Error: variable __r54 cannot be read at compile time
ctfe.d(5):        called from here: _d_arraycopyT(__r54, s[])
ctfe.d(15):        called from here: f(['z', 'b', 'c', '.', 'd'])
ctfe.d(15):        while evaluating pragma(msg, f(['z', 'b', 'c', '.', 'd']))

-------------------------------------------------------

It looks like the problem is with the foreach loop. In the example above, the (0) and (1) loops result in the compilation error mentioned. Loop (2) with dchar is fine.

I also tried using a regular for loop and this one compiled.