Possible bug when instantiating template function with nested struct

[Teodor Dutu]

Hi,

As part of my project for SAoC 2021, I am trying to rewrite the 
hook to `_d_arrayctor` to a template function. This work was 
started by [Dan Printzell](https://github.com/Vild/) 2 years ago. 
Dan managed to implement a template version of `_d_arrayctor` in 
druntime and took some steps to switch to this version in dmd as 
well:
- https://github.com/dlang/druntime/pull/2655.
- https://github.com/dlang/dmd/pull/10102

In essence, Dan performs the following lowering:
```d
T[2] a;
T[2] b = a;
// is lowered to:
T[2] a;
T[2] b;
_darrayctor(b[], a[]);
```

After picking up Dan's work, I brought [some 
fixes](https://github.com/dlang/dmd/pull/13116) to his changes to 
dmd, so that the code now passes the tests in dmd and druntime. 
However, a few warnings issued by some tests in phobos revealed a 
flaw in `_d_arrayctor`'s signature. Some time ago, I wrote [a 
forum 
post](https://forum.dlang.org/post/simesvkancmscrtsciwq@forum.dlang.org) about this issue.

It goes something like this: when lowered using `const` or 
`immutable` arguments, `_d_arrayctor` becomes strongly pure. For 
instance, in the code snippet below,
```d
struct S {};
const S[2] b;
const S[2] a = b;
```
the line `const S[2] a = b;` is lowered to `_d_arrayctor(a[], 
b[])`, which is the intended behaviour.

But, since, in this case, `_d_arrayctor` is strongly pure and 
since its return value is ignored, the compiler issues the 
warning in the test mentioned above. In addition, its strong 
purity might also cause calls to `_d_arrayctor` to be removed by 
the compiler as part of the optimisation phase.

My mentors and I first tried changing the type of either `to` or 
the `from` parameters to a mutable `void[]`, but in this case the 
compiler was unable to instantiate the function's template 
correctly. So this solution didn't work.

The only alternative we could initially come up with was to force 
`_d_arrayctor` to be weakly pure instead. We achieved this by 
adding a third, unused, pointer-type parameter, as implemented in 
[this PR](https://github.com/dlang/druntime/pull/3587), which 
changes `_d_arrayctor`'s signature to:
```d
Tarr _d_arrayctor(Tarr : T[], T)(return scope Tarr to, scope Tarr 
from, char* makeWeaklyPure = null) @trusted
```
But this is merely a stop-gap solution, because it acts against 
the language, by denying one of its properties: purity.

Razvan asked around and found a new, more elegant approach: 
change the signature of `_d_arrayctor` so that it creates the 
destination array itself, instead of simply modifying it. This 
will make use of the function's return value, thus removing the 
warnings from phobos that I mentioned above. So the lowering 
would be changed to something like:
```d
T[2] a;
T[2] b = a;
// would be roughly lowered to:
T[2] a;
T[2] b;
b = _darrayctor(a[], b.length);
```
The point of this approach is to make use of NRVO so that the 
contents of `b` are initialised directly inside `_d_arrayctor`, 
whithout having to copy them back to `b` after the function call.

But even this idea ran into trouble, as, by giving `b.length` as 
a function parameter, the constructed array can only be a dynamic 
array. As this lowering is only performed for static arrays, NRVO 
is not possible. The code works, the tests pass, but it's 
inefficient, thus possibly making the entire replacement of the 
runtime hook kind of useless. A working implementation of this 
approach can be found 
[here](https://github.com/teodutu/druntime/blob/38cbc346cbb0142707161b1dce3ab22187c80dad/src/core/internal/array/construction.d).

Then we had several attempts to create the returned array 
statically inside the scope of `_d_arrayctor` in the hope of 
triggering NRVO. The first was to pass the length of the newly 
created array as another template parameter, like so (note that 
this would create new a template instance for every different 
length of the created array and that's a lot of code):
```d
T[] _d_arrayctor(Tarr : T[], T, size_t length)(scope Tarr from) 
@trusted
{
     // ...
     T[length] to;
     // ...
     return to;  // this is line 83
}
```
As you've probably guessed, this code does not compile because it 
returns a reference to a local variable: `to`:
```
src/core/internal/array/construction.d(83): Error: returning 
`cast(S[])to` escapes a reference to local variable `to`
```

So I tried changing the function's signature once again:
```d
Tarr1 _d_arrayctor(Tarr1 : T1[], Tarr2 : T2[], T1, T2)(scope 
Tarr2 from) @trusted
{
     // ...
     Tarr1 to;  // this is line 35
     // ...
     return to;
}
```
This attempt binds `Tarr1` to a static array. In the case of 
[this 
unittest](https://github.com/dlang/druntime/blob/69ba1f900733f9929d3f704c9c66d393806a343b/src/core/internal/array/construction.d#L84-L99), for example, `Tarr` is bound to `S[4]`, after changing the call to `_d_arrayctor` to:
```d
arr1 = _d_arrayctor!(typeof(arr1), typeof(arr2))(arr2[]);
```
This should have worked, but instead, the compiler raises a 
rather strange error:
```
src/core/internal/array/construction.d(35): Error: cannot access 
frame pointer of 
`core.internal.array.construction.__unittest_L87_C7.S`
```

The error seems to have something to do with `struct S` which is 
used to instantiate the template. When changing it and `counter` 
to `static`, the unittest passes.

Finally, my question is related to this error. Is this normal 
behaviour? Razvan and I tried to search for this bug and found 
these two issues:
- https://issues.dlang.org/show_bug.cgi?id=8850
- https://issues.dlang.org/show_bug.cgi?id=8863

[This comment](https://issues.dlang.org/show_bug.cgi?id=8863#c2) 
points out that indeed this is a bug. Or, at least, it was back 
in 2012. It was [supposedly 
fixed](https://github.com/dlang/dmd/commit/c6a41c414aa9177aef7dac6ac131addba20abf32), but it still manifests itself.

What could I do at this point?

Thanks,
Teodor