[WIP] core.array.staticArray static array litteral: `staticArray(1, 2, 3)` #2093

timotheecour · 2018-02-14T21:20:06Z

usage

auto a = staticArray(1, 2, 3);
float[2] = staticArray!float(1, 2);

motivation

places like here: dlang/dub#1377 (comment)

notes

NOTE: in core.runtime so it can be used in more settings including dmd compiler which can't use phobos
NOTE: other (stalled) PR's have proposed something like staticArray(T)(T[] a) ; I suggest this could be done instead (in another PR) via (something like) toStaticArray(T)(T[]a) to avoid name ambiguity

links

https://issues.dlang.org/show_bug.cgi?id=8008 Issue 8008 - Syntax for fixed size array literals like [1,2,3]s (edit)
https://issues.dlang.org/show_bug.cgi?id=481 Issue 481 - Fixed-length arrays with automatically computed length (edit)
dlang/phobos#4936 Fix Issue 16745 - Add template helper for creating static arrays with the size inferred #4936
dlang/phobos#4090 Better static array support in std.array #4090

not related

https://issues.dlang.org/show_bug.cgi?id=9165 Issue 9165 - Auto conversion from dynamic array to fixed size array at return (edit)

TODO

update the dreadful mak/*
decide whether to put in std.array or core.array
decide whether both staticArray and asStatic are needed or whether asStatic is sufficient
probably need to add asStaticCast as well for symmetry (is there a better way to avoid the code dup?)
should i make rename staticArrayCast as staticArray so that there's no user-looking difference bw case where an explicit cast is needed vs not.

dlang-bot · 2018-02-14T21:20:08Z

Thanks for your pull request and interest in making D better, @timotheecour! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

My PR is fully covered with tests (you can see the annotated coverage diff directly on GitHub with CodeCov's browser extension
My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
I have provided a detailed rationale explaining my changes
New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

wilzbach · 2018-02-14T23:25:13Z

src/core/array.d

+    return [a];
+}
+
+unittest


Make the unittest public, s.t. it's visible on the docs.

Also add @nogc @safe nothrow and pure. A static array should be usable in the full set.

Also add @nogc @safe nothrow and pure. A static array should be usable in the full set.

done

Make the unittest public, s.t. it's visible on the docs.

not done yet

Make the unittest public, s.t. it's visible on the docs.

done

wilzbach · 2018-02-14T23:25:23Z

src/core/array.d

+/++
+    Returns a static array constructed from `a`. The type of elements can be
+    specified implicitly (`int[2] a = staticArray(1,2)`) or explicitly
+    (`float[2] a = staticArray!float(1,2)`).


Params + Returns

i think this is too much boilerplate and doesn't add anything in this case; looking at https://dlang.org/phobos/std_array.html#array , functions where it's obvious from rest of ddoc don't do it either.

wilzbach · 2018-02-14T23:26:32Z

src/core/array.d

+        auto a = staticArray(1, 2.0);
+        assert(is(typeof(a) == double[2]));
+        assert(a == [1, 2.0]);
+    }


Simply create new unittest blocks instead of scoping. It doesn't hurt and allows the user of the documentation to hit "run" and "edit" for an individual block. Plus you can give a short summary title in /// Amazing example

pushing back on this request as well, but happy to revisit if you have good arguments

creates less visual clutter to group (and takes less vertical space both in rendered doc and in source code); the inline comments will still be visible in rendered docs

(maybe this one isn't an important point) potentially less executable / compile time / runtime bloat

wilzbach · 2018-02-14T23:28:10Z

src/core/array.d

+}
+
+package:
+// copied from std.traits ; TODO: move to core.internal.adapted?


Well, imho std.traits and std.meta should have been part of druntime, but yeah for nor core.internal.traits is the place to dump traits from Phobos.

wilzbach · 2018-02-14T23:35:17Z

Missing comments from Andrei:

Add an overload that takes a range and is meant to be evaluated during compilation
Add documentation explaining the tradeoffs of the rvalue return
add constraint if (is(T : TargetElemT))

jmdavis · 2018-02-14T23:39:20Z

Well, this implementation is going to have problems with VRP, but until https://issues.dlang.org/show_bug.cgi?id=16779 is fixed, we don't have a way around that.

Regardless though, this isn't the sort of thing that normally goes in druntime. Typically, only the stuff that needs to go in druntime goes in druntime, and this isn't needed in druntime. It belongs in Phobos.

timotheecour · 2018-02-15T02:14:48Z

done:Add an overload that takes a range and is meant to be evaluated during compilation
done:Add documentation explaining the tradeoffs of the rvalue return
done:add constraint if (is(T : TargetElemT))

Regardless though, this isn't the sort of thing that normally goes in druntime. Typically, only the stuff that needs to go in druntime goes in druntime, and this isn't needed in druntime. It belongs in Phobos.

I was hesitationg as well. Would be useful to have access to it in compiler, which uses druntime and not phobos. Let's delay that decision until rest of PR is good so i don't keep moving files around

skl131313 · 2018-02-15T03:52:54Z

I was hesitationg as well. Would be useful to have access to it in compiler, which uses druntime and not phobos. Let's delay that decision until rest of PR is good so i don't keep moving files around

All of phobos would be useful to have access in the compiler. Should everything be backported to druntime then? If the answer is no, then this shouldn't be included either. If you want access to phobos-like features then an effort should be be made into getting phobos running in the compiler. Don't know DMD's structure that well but if it isn't possible then a separate library that only DMD uses should be created instead.

wilzbach · 2018-02-15T04:10:33Z

If you want access to phobos-like features then an effort should be be made into getting phobos running in the compiler.

There's no effort required for this, it's a deliberate decision from Walter to explicitly avoid using Phobos. Phobos could already be used at dmd - in fact the HOST_DMD even links with it, but the linker strips it out as it's not used.

skl131313 · 2018-02-15T04:16:48Z

Oh I thought it wasn't used cause it wasn't possible. Then this definitely shouldn't go into druntime. If features like these are desired in DMD then someone needs to convince for phobos to be used in DMD.

timotheecour · 2018-02-15T04:18:42Z

ok @skl131313 u convinced me, will reopen a PR after all these issues are cleared

cc @WalterBright

someone needs to convince for phobos to be used in DMD

yes! I don't understand the decision for dmd not being allowed to use phobos btw, that just creates all this ugly guesswork and copies in core.internal, and makes dmd sub-optimal and verbose in many places. Arguments against need to be clearly written down so they can be fought

skl131313 · 2018-02-15T04:23:51Z

src/core/array.d

+    specified implicitly (`int[2] a = staticArray(1,2);`) or explicitly
+    (`float[2] a = staticArray!float(1,2)`).
+
+    The result is an rvalue, therefore uses like staticArray(1, 2, 3).find(x) may be inefficient.


Maybe providing some links to what an rvalue is or explaining the inefficiency a bit better. Just from the stand point of someone that isn't familiar with what rvalues are and why this might be inefficient. Using find(staticArray(...), x) might better illustrate that point as well.

Maybe providing some links to what an rvalue is

i think that's out of scope; other functions mentioning rvalue don't explain either

explaining the inefficiency a bit better.

done

find(staticArray(...), x)

changed it to fun(staticArray(...))

timotheecour · 2018-02-15T04:28:21Z

@jmdavis

Well, this implementation is going to have problems with VRP, but until https://issues.dlang.org/show_bug.cgi?id=16779 is fixed, we don't have a way around that.

added staticArrayCast which allows casting

timotheecour · 2018-02-15T05:15:39Z

after feedback from reviewers, moved to a new PR under phobos: dlang/phobos#6178

timotheecour added 3 commits February 14, 2018 12:45

core.array.StaticArray

1e6c101

fixup

478eac0

fixup

e51f829

timotheecour requested review from andralex and wilzbach as code owners February 14, 2018 21:20

dlang-bot added the WIP Work In Progress - not ready for review or pulling label Feb 14, 2018

timotheecour mentioned this pull request Feb 14, 2018

Fix 1037 (rebased against master) dlang/dub#1377

Merged

wilzbach reviewed Feb 14, 2018

View reviewed changes

wilzbach mentioned this pull request Feb 14, 2018

Fix Issue 16745 - Add template helper for creating static arrays with the size inferred dlang/phobos#4936

Closed

added asStatic

111642b

timotheecour force-pushed the pr_staticarray branch from db4799f to 111642b Compare February 15, 2018 01:43

timotheecour added 2 commits February 14, 2018 18:00

added asStatic!(2.iota)

462779a

fixup

0c58d16

timotheecour added 3 commits February 14, 2018 18:17

fixup

4f79a3b

fixup

7580600

refactor

bcf1ee3

timotheecour requested review from MartinNowak and schveiguy as code owners February 15, 2018 02:40

fixup

2e140ea

skl131313 reviewed Feb 15, 2018

View reviewed changes

fixup

2a3ca26

timotheecour added 2 commits February 14, 2018 20:39

fixup

69d0176

fixup

588ed37

timotheecour mentioned this pull request Feb 15, 2018

static array litteral: [1,2].asStatic + overloads for CT ranges, RT ranges, explicit types dlang/phobos#6178

Closed

timotheecour closed this Feb 15, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] core.array.staticArray static array litteral: `staticArray(1, 2, 3)` #2093

[WIP] core.array.staticArray static array litteral: `staticArray(1, 2, 3)` #2093

timotheecour commented Feb 14, 2018 •

edited

Loading

dlang-bot commented Feb 14, 2018

wilzbach Feb 14, 2018

wilzbach Feb 14, 2018

timotheecour Feb 15, 2018

timotheecour Feb 15, 2018 •

edited

Loading

wilzbach Feb 14, 2018

timotheecour Feb 15, 2018

wilzbach Feb 14, 2018

timotheecour Feb 15, 2018

wilzbach Feb 14, 2018

timotheecour Feb 15, 2018

wilzbach commented Feb 14, 2018

jmdavis commented Feb 14, 2018

timotheecour commented Feb 15, 2018

skl131313 commented Feb 15, 2018

wilzbach commented Feb 15, 2018

skl131313 commented Feb 15, 2018

timotheecour commented Feb 15, 2018 •

edited

Loading

skl131313 Feb 15, 2018

timotheecour Feb 15, 2018 •

edited

Loading

timotheecour commented Feb 15, 2018

timotheecour commented Feb 15, 2018

[WIP] core.array.staticArray static array litteral: staticArray(1, 2, 3) #2093

[WIP] core.array.staticArray static array litteral: staticArray(1, 2, 3) #2093

Conversation

timotheecour commented Feb 14, 2018 • edited Loading

usage

motivation

notes

links

not related

TODO

dlang-bot commented Feb 14, 2018

Bugzilla references

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timotheecour Feb 15, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wilzbach commented Feb 14, 2018

jmdavis commented Feb 14, 2018

timotheecour commented Feb 15, 2018

skl131313 commented Feb 15, 2018

wilzbach commented Feb 15, 2018

skl131313 commented Feb 15, 2018

timotheecour commented Feb 15, 2018 • edited Loading

Choose a reason for hiding this comment

timotheecour Feb 15, 2018 • edited Loading

Choose a reason for hiding this comment

timotheecour commented Feb 15, 2018

timotheecour commented Feb 15, 2018

[WIP] core.array.staticArray static array litteral: `staticArray(1, 2, 3)` #2093

[WIP] core.array.staticArray static array litteral: `staticArray(1, 2, 3)` #2093

timotheecour commented Feb 14, 2018 •

edited

Loading

timotheecour Feb 15, 2018 •

edited

Loading

timotheecour commented Feb 15, 2018 •

edited

Loading

timotheecour Feb 15, 2018 •

edited

Loading