-
-
Notifications
You must be signed in to change notification settings - Fork 707
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
static array litteral: [1,2].asStatic
+ overloads for CT ranges, RT ranges, explicit types
#6178
Changes from all commits
3573814
5d55c90
23fa8a8
b36ecb9
ce8e5df
3e881b2
01187db
1a4dbc5
2f9fd51
b9d44ed
fc54fe5
0050968
a965ed7
6519a2e
3cd479b
aa082d4
25114ab
5e318a8
c19751a
d7444e7
1066a2f
dbc94b0
60d6cd6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
Added `staticArray` to construct a static array from input array / range / CT range. | ||
|
||
The type of elements can be specified implicitly (`[1,2].staticArray` of type int[2]) | ||
or explicitly (`[1,2].staticArray!float` of type float[2]). | ||
When `a` is a range (not known at compile time), the number of elements has to be given as template argument | ||
(eg `myrange.staticArray!2`). | ||
Size and type can be combined (eg: `2.iota.staticArray!(byte[2])`). | ||
When the range `a` is known at compile time, it can also be specified as a | ||
template argument to avoid having to specify the number of elements | ||
(eg: `staticArray!(2.iota)` or `staticArray!(double, 2.iota)`). | ||
|
||
Note: `foo([1, 2, 3].staticArray)` may be inefficient because of the copies involved. | ||
|
||
--- | ||
auto a1 = [0, 1].staticArray; | ||
static assert(is(typeof(a1) == int[2])); | ||
assert(a1 == [0, 1]); | ||
|
||
auto a2 = [0, 1].staticArray!byte; | ||
static assert(is(typeof(a2) == byte[2])); | ||
|
||
import std.range : iota; | ||
auto input = 2.iota; | ||
auto a3 = input.staticArray!2; | ||
auto a4 = input.staticArray!(byte[2]); | ||
|
||
auto a5 = staticArray!(2.iota); | ||
auto a6 = staticArray!(double, 2.iota); | ||
--- |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3927,3 +3927,218 @@ unittest | |
assert(appS.data == "hellow"); | ||
assert(appA.data == "hellow"); | ||
} | ||
|
||
/++ | ||
Constructs a static array from `a`. | ||
The type of elements can be specified implicitly (`[1,2].staticArray` of type int[2]) | ||
or explicitly (`[1,2].staticArray!float` of type float[2]). | ||
When `a` is a range (not known at compile time), the number of elements has to be given as template argument | ||
(e.g. `myrange.staticArray!2`). | ||
Size and type can be combined (eg: `2.iota.staticArray!(byte[2])`). | ||
When the range `a` is known at compile time, it can also be specified as a | ||
template argument to avoid having to specify the number of elements | ||
(eg: `staticArray!(2.iota)` or `staticArray!(double, 2.iota)`). | ||
|
||
Note: `staticArray` returns by value, so expressions involving large arrays may be inefficient. | ||
|
||
Params: a = The input elements | ||
|
||
Returns: A static array constructed from `a`. | ||
+/ | ||
|
||
pragma(inline, true) T[n] staticArray(T, size_t n)(auto ref T[n] a) nothrow @safe pure @nogc | ||
{ | ||
return a; | ||
} | ||
|
||
/// ditto | ||
U[n] staticArray(U, T, size_t n)(auto ref T[n] a) nothrow | ||
if (!is(U == T)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Template constraint should include that T can be cast to U. |
||
{ | ||
import std.conv : emplaceRef; | ||
|
||
U[n] ret = void; | ||
// TODO: ElementType vs ForeachType | ||
static foreach (i; 0 .. n) | ||
{ | ||
emplaceRef!U(ret[i], cast(U) a[i]); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A cast here is dangerous, especially since it's virtually undocumented. I understand the necessity is to create arrays of I'll "ask for changes" on account of this. I recall you had a variant This is the kind of decision that can be deferred - I suggest you remove this overload entirely, get this in, then add something later. |
||
} | ||
return ret; | ||
} | ||
|
||
/// static array from array | ||
nothrow pure @safe unittest | ||
{ | ||
auto a = [0, 1].staticArray; | ||
static assert(is(typeof(a) == int[2])); | ||
assert(a == [0, 1]); | ||
|
||
auto b = [0, 1].staticArray!byte; | ||
static assert(is(typeof(b) == byte[2])); | ||
assert(b == [0, 1]); | ||
} | ||
|
||
nothrow pure @safe unittest | ||
{ | ||
int val = 3; | ||
static immutable gold = [1, 2, 3]; | ||
[1, 2, val].staticArray.checkStaticArray!int([1, 2, 3]); | ||
|
||
@nogc void checkNogc() | ||
{ | ||
[1, 2, val].staticArray.checkStaticArray!int(gold); | ||
} | ||
|
||
checkNogc(); | ||
|
||
[1, 2, val].staticArray!double.checkStaticArray!double(gold); | ||
[1, 2, 3].staticArray!int.checkStaticArray!int(gold); | ||
|
||
[1, 2, 3].staticArray!(const(int)).checkStaticArray!(const(int))(gold); | ||
[1, 2, 3].staticArray!(const(double)).checkStaticArray!(const(double))(gold); | ||
{ | ||
const(int)[3] a2 = [1, 2, 3].staticArray; | ||
} | ||
|
||
[1, 129].staticArray!byte.checkStaticArray!byte([1, -127]); | ||
|
||
} | ||
|
||
/// ditto | ||
auto staticArray(size_t n, T)(T a) | ||
if (isInputRange!T) | ||
{ | ||
alias U = ElementType!T; | ||
return staticArray!(U[n], U, n)(a); | ||
} | ||
|
||
/// ditto | ||
auto staticArray(Un : U[n], U, size_t n, T)(T a) nothrow | ||
if (isInputRange!T) | ||
{ | ||
import std.conv : emplaceRef; | ||
|
||
U[n] ret = void; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same discussion of nothrow |
||
size_t i; | ||
foreach (ref ai; a) | ||
{ | ||
emplaceRef!U(ret[i++], cast(U) ai); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. DEFINITELY no cast here |
||
} | ||
assert(i == n); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is definitely unsatisfactory. Code shouldn't rely on bounds checking to be correct (see https://dlang.org/spec/arrays.html#bounds), and by the time the assert is even looked at the code has trampled well into infested territory. Too short is not good, either, because there'd be portions of the array that are left uninitialized! This function is thoroughly problematic. One solution I suggested to @n8sh: define an overload taking an auto staticArray(Un : U[n], U, size_t n, T)(T a, out size_t filled)
{
U[n] result = void;
size_t i = 0;
// same exception safety mechanism as above
foreach (ref ai; a)
{
if (i == n) break;
emplace(...);
}
filled = i;
return result;
}
auto staticArray(Un : U[n], U, size_t n, T)(auto ref T a)
{
size_t unused;
return staticArray!(Un, U, n, T)(a, unused);
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Forgot to mention, some ranges return an rvalue from |
||
return ret; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This code duplicates implementation (and bug) from a different overload. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
} | ||
|
||
/// static array from range + size | ||
nothrow pure @safe unittest | ||
{ | ||
import std.range : iota; | ||
|
||
auto input = 2.iota; | ||
auto a = input.staticArray!2; | ||
static assert(is(typeof(a) == int[2])); | ||
assert(a == [0, 1]); | ||
auto b = input.staticArray!(byte[2]); | ||
static assert(is(typeof(b) == byte[2])); | ||
assert(b == [0, 1]); | ||
} | ||
|
||
nothrow pure @safe unittest | ||
{ | ||
|
||
auto a = [1, 2].staticArray; | ||
assert(is(typeof(a) == int[2]) && a == [1, 2]); | ||
|
||
import std.range : iota; | ||
|
||
2.iota.staticArray!2.checkStaticArray!int([0, 1]); | ||
2.iota.staticArray!(double[2]).checkStaticArray!double([0, 1]); | ||
2.iota.staticArray!(byte[2]).checkStaticArray!byte([0, 1]); | ||
} | ||
|
||
nothrow pure @system unittest | ||
{ | ||
import std.range : iota; | ||
|
||
assert(isThrown!Error(2.iota.staticArray!1)); | ||
assert(isThrown!Error(2.iota.staticArray!3)); | ||
|
||
// NOTE: correctly issues a deprecation | ||
// int[] a2 = [1, 2].staticArray; | ||
} | ||
|
||
/// ditto | ||
auto staticArray(alias a)() | ||
if (isInputRange!(typeof(a))) | ||
{ | ||
// NOTE: without `cast`, getting error: | ||
// cannot deduce function from argument types !(length)(Result) | ||
return .staticArray!(cast(size_t) a.length)(a); | ||
} | ||
|
||
/// ditto | ||
auto staticArray(U, alias a)() | ||
if (isInputRange!(typeof(a))) | ||
{ | ||
return .staticArray!(U[cast(size_t) a.length])(a); | ||
} | ||
|
||
/// static array from CT range | ||
nothrow pure @safe unittest | ||
{ | ||
import std.range : iota; | ||
|
||
enum a = staticArray!(2.iota); | ||
static assert(is(typeof(a) == int[2])); | ||
assert(a == [0, 1]); | ||
|
||
enum b = staticArray!(byte, 2.iota); | ||
static assert(is(typeof(b) == byte[2])); | ||
assert(b == [0, 1]); | ||
} | ||
|
||
nothrow pure @safe unittest | ||
{ | ||
import std.range : iota; | ||
|
||
enum a = staticArray!(2.iota); | ||
staticArray!(2.iota).checkStaticArray!int([0, 1]); | ||
staticArray!(double, 2.iota).checkStaticArray!double([0, 1]); | ||
staticArray!(byte, 2.iota).checkStaticArray!byte([0, 1]); | ||
} | ||
|
||
version(unittest) private void checkStaticArray(T, T1, T2)(T1 a, T2 b) nothrow @safe pure @nogc | ||
{ | ||
static assert(is(T1 == T[T1.length])); | ||
assert(a == b); | ||
} | ||
|
||
/+ | ||
TODO: consider adding this to assertThrown in std.exception | ||
The alternatives are not nothrow (see https://issues.dlang.org/show_bug.cgi?id=12647) | ||
--- | ||
import std.exception : assertThrown; | ||
assertThrown!Error(expr_that_throws); | ||
|
||
// or | ||
import std.exception : ifThrown; | ||
scope(failure) assert(0); | ||
assert(ifThrown!Error({ expr_that_throws; return false; }(), true)); | ||
--- | ||
Also, `isThrown` is easier to use than ifThrown and more flexible than assertThrown. | ||
+/ | ||
version(unittest) private bool isThrown(T : Throwable = Exception, E)(lazy E expression) nothrow | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nice There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ack |
||
{ | ||
try | ||
{ | ||
expression(); | ||
return false; | ||
} | ||
catch (T) | ||
{ | ||
return true; | ||
} | ||
catch (Exception) | ||
{ | ||
return false; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd accept the argument "let's leave it nothrow and worry about it later) if (a) this PR were urgent, and/or (b) the right code would be difficult to implement. Neither is applicable. The right code is (stylized):
That's awkward and disables named return value optimization, so here's what I think would be a better approach:
This is cute and hopefully not too much so - it comes from the opposite direction: in case of an exception, it initializes (in a nothrow manner) the uninitialized part of the result and then leaves the rest to the compiler. This allows NRVO, is efficient on the nothrow path, and is simple.