Fix Issue 16745 - Add template helper for creating static arrays with the size inferred

[John-Colvin]

No description provided.

[dlang-bot]

Thanks for your pull request, @John-Colvin! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.

Some tips to help speed things up:

• smaller, focused PRs are easier to review than big ones

• try not to mix up refactoring or style changes with bug fixes or feature enhancements

• provide helpful commit messages explaining the rationale behind each change

Bear in mind that large or tricky changes may require multiple rounds of review and revision.

Please see CONTRIBUTING.md for more information.

Bugzilla references

Auto-close	Bugzilla	Description
✓	16745	Add template helper for creating static arrays with the size inferred

[wilzbach]

This has been tried before: #4090, see especially Andrei's closing comment

[jmdavis]

I don't know why you're using emplaceRef. I believe that it makes it so that this can't be @nogc, and

T[n] staticArray(T, size_t n)(auto ref T[n] arr)
{
    return arr;
}

should be equivalent. However, regardless of that, I'm pretty sure that your solution doesn't work with VRP thanks to https://issues.dlang.org/show_bug.cgi?id=16779. So, something like

auto sa = staticArray!ubyte([1, 2, 3, 4]);

wouldn't work, whereas

ubyte[4] sa = [1, 2, 3, 4];

does. Also, there's the question of whether it works with runtime variables with the equivalent of

int i = 7;
int[4] arr = [1, 2, 3, i];

which I think that what you have does do, though you're not testing for it.

I think that any solution for this should be @nogc, work with VRP, and allow runtime variables, because you can do all those things when you initialize the static array directly with a known size. Based on a recent D.Learn discussion on the topic, the best solution seem to be

template staticArray(T, E...)
{
    pragma(inline, true) T[E.length] staticArray() @property { return [E]; }
}
template staticArray(E...)
{
    pragma(inline, true) typeof([E][0])[E.length] staticArray() @property { return [E]; }
}

which works with VRP properly, unlike what you have right now. It doesn't have a range overload like what you have now, but that's easily fixed. The bigger problem would be that it doesn't work with what you're proposing with dynamic arrays (which is something that you can do with direct initialization and therefore really should work). And it's not going to work with dynamic arrays without an overload that takes the dynamic array as a function argument rather than a template argument. That being the case,

T[n] staticArray(T, size_t n)(auto ref T[n] arr)
{
    return arr;
}

would be a better solution so long as https://issues.dlang.org/show_bug.cgi?id=16779 gets fixed.

[jmdavis]

This has been tried before: #4090, see especially Andrei's closing comment

So, is his big concern then that if you do something like

int[] arr = staticArray([1, 2, 3, 4]);

you have memory problems? I think that a fix for https://issues.dlang.org/show_bug.cgi?id=12625 would fix that problem, because slicing a static array that's an rvalue should always be an error. As such, I don't think that it's worth worrying about as far as adding a function like this to Phobos goes.

[John-Colvin]

I don't know why you're using emplaceRef. I believe that it makes it so that this can't be @nogc, and

T[n] staticArray(T, size_t n)(auto ref T[n] arr)
{
   return arr;
}

should be equivalent.

I am using it because in that overload the user can specify a TargetElemT so we need to place each element in to a new type, just like std.array.array does for handling qualified types. I have added a new test that shows it is @nogc.

However, regardless of that, I'm pretty sure that your solution doesn't work with VRP thanks to https://issues.dlang.org/show_bug.cgi?id=16779. So, something like

auto sa = staticArray!ubyte([1, 2, 3, 4]);

wouldn't work, whereas

ubyte[4] sa = [1, 2, 3, 4];

does

Correct, that's a pain. Maybe it will get fixed soon?

Also, there's the question of whether it works with runtime variables with the equivalent of

int i = 7;
int[4] arr = [1, 2, 3, i];

which I think that what you have does do, though you're not testing for it.

Added a test for that.

I've been experimenting with different designs, but I keep being thwarted by https://issues.dlang.org/show_bug.cgi?id=16957. Seeing as some of the errors in there disappear when not inferring static array length, perhaps it's also linked to vrp problem.

[jmdavis]

I am using it because in that overload the user can specify a TargetElemT so we need to place each element in to a new type, just like std.array.array does for handling qualified types.

Hmmm. Well, that seems like it adds a fair bit of extra overhead for a case that does not work with normal static array initialization. The

int[] arr = [1, 2, 3, 4];
const(int)[2] sa = arr[1 .. $ - 1];

case works and would work with staticArray without emplaceRef. It's just

int[] arr = [1, 2, 3, 4];
long[2] sa = arr[1 .. $ - 1];

that wouldn't. I'm inclined to argue that we should be trying to get staticArray to function as much like normal static array initialization as possible save for the fact that it infers the length, and choosing a solution with additional overhead to support other uses cases doesn't seem like a good idea to me.

[wilzbach]

I'm inclined to argue that we should be trying to get staticArray to function as much like normal static array initialization as possible save for the fact that it infers the length, and choosing a solution with additional overhead to support other uses cases doesn't seem like a good idea to me.

I absolutely agree here. I think the most common use case is just this:

[1, 2, 3].asStatic

(for which btw the name asStatic would play very nicely with UFCS).

would be a better solution so long as https://issues.dlang.org/show_bug.cgi?id=16779 gets fixed.

So what's the consensus here?

A) Use this PR
B) Wait until 16779 gets fixed for full dynamic array support
C) Don't allow the user to pass in a custom type without length (as it's restricted by 16779)

[MetaLang]

@John-Colvin since this has been open since December of last year without getting merged, and since Andrei closed the previous PR, I'm going to close this one as well. @andralex pleas re-open if you've changed your mind.

[andralex]

OK, I wasn't being passive-aggressive here, this just slipped. I'll add a few comments within.

[John-Colvin]

Ok, no problem. I personally don't normally care much about waiting, although of course "good for the project" and all that...

I've rebased to fix the conflicts

[andralex]

staticArray with a range (including an array) seems to be sufficient. The documentation should dedicate a paragraph to the fact that the result is an rvalue, therefore uses like [1, 2, 3].staticArray.find(x) may be inefficient.

[andralex]

This strikes me as an obscure feature, and the example doesn't add confidence in the usefulness of the approach. I'd think a CTFE range evaluation should work, i.e. the example above should spell like:

auto sa4 = iota(3).staticArray;
assert(is(typeof(sa4) == int[3]));
assert(sa4 == [0, 1, 2]);

[wilzbach]

This doesn't work as of now. See also:

https://wiki.dlang.org/User:Quickfur/Compile-time_vs._compile-time

[andralex]

perhaps you could add auto ref to the parameter

[andralex]

add constraint if (is(T : TargetElemT))

[andralex]

and possibly auto ref to the parameter

[andralex]

If any of these throws, the destructors of the already-constructed elements is not called.

[andralex]

I'm not clear why this should be always trusted.

[andralex]

Examples should use the preferred notation [1, 2].staticArray.

[andralex]

To summarize, the changes I'm suggesting are:

• Delete the overload taking an alias
• Add an overload that takes a range and is meant to be evaluated during compilation
• Add documentation explaining the tradeoffs of the rvalue return

[wilzbach]

@timotheecour has revived this to DRuntime: dlang/druntime#2093