infer elementType for ranges in makeArray,makeSlice #4263

wilzbach · 2016-04-29T14:28:56Z

As discussed earlier, it's possible to infer the ElementType for makeArray with Ranges.
Even better we can keep full compatibility and make this feature opt-in - is anyone against this?

tl;dr: let's allow the variant b! (a is still possible)

auto a = alloc.makeArray!long([5, 42]);
auto b = alloc.makeArray([5, 42]);

Ping @9il @andralex

edit: attached https://issues.dlang.org/show_bug.cgi?id=16450 to it.

9il · 2016-04-29T14:30:47Z

std/experimental/allocator/package.d


 /// Ditto
-T[] makeArray(T, Allocator, R)(auto ref Allocator alloc, R range)
+auto makeArray(T = void, Allocator, R)(auto ref Allocator alloc, R range)


Why not T = ElementType!Range?

I would prefer that, but

this might break compatibility (which might be okay because it's experimental)

removes the feature to use a different type

auto makeArray(T = ElementType!R, Allocator, R)
1 and 2. really?

wilzbach · 2016-04-29T15:19:31Z

thanks to @9il helpful I removed T = void and replaced it with an additional overload without T.

9il · 2016-04-29T15:22:45Z

std/experimental/allocator/package.d

+/// Ditto
+Unqual!(ElementType!R)[] makeArray(Allocator, R)(auto ref Allocator alloc, R range)
+if (isInputRange!R)
+{


.primitives

PetarKirov · 2016-04-29T15:56:07Z

I also prefer Ilya's version:
auto makeArray(T = ElementType!R, Allocator, R)

Doesn't break the API.
Still allows the user to specify the type manually.

wilzbach · 2016-04-29T15:59:30Z

auto makeArray(T = ElementType!R, Allocator, R)

I would prefer that too, but unfortunately D isn't that smart. It yields R is not defined.

MetaLang · 2016-05-03T23:38:19Z

@wilzbach that should work. It may be a possible bug with template argument type deduction.

wilzbach · 2016-05-06T11:02:11Z

@wilzbach that should work. It may be a possible bug with template argument type deduction.

@MetaLang AFAIK it's a missing feature, which would be awesome to have!
See Issue 12433

andralex · 2016-06-18T08:41:43Z

std/experimental/allocator/package.d

+Unqual!(ElementType!R)[] makeArray(Allocator, R)(auto ref Allocator alloc, R range)
+if (isInputRange!R)
+{
+    import std.range.primitives;


Use ElementEncodingType?

ElementEncodingType:

static if (is(StringTypeOf!R) && is(R : E[], E)) alias ElementEncodingType = E; else alias ElementEncodingType = ElementType!R

ElementType:

static if (is(typeof(R.init.front.init) T)) alias ElementType = T; else alias ElementType = void;

... where is the difference?

ElementType!(char[]) == dchar
ElementEncodingType!(char[]) == char

(Because R.init.front.init is dchar for narrow strings.)

andralex · 2016-06-18T08:44:54Z

preapproved once nits are looked at

wilzbach · 2016-06-19T21:40:31Z

(Because R.init.front.init is dchar for narrow strings.)

Ah auto-decoding :S
Thanks for the explanation @ZombineDev!
(Rebased)

PetarKirov · 2016-06-19T23:42:56Z

No problem :)

Maybe not directly related to your PR, but do you happen to know what will makeArray return if the elements of the range are classes - an array of references to class instances or the actual array where the objects were allocated and emplaced?

JackStouffer · 2016-06-29T15:09:35Z

Ping @9il, I think this can be merged now

9il · 2016-06-29T20:45:11Z

std/experimental/allocator/package.d

 }

+/// Ditto
+Unqual!(ElementType!R)[] makeArray(Allocator, R)(auto ref Allocator alloc, R range)


&& !infinityRange!R

wilzbach · 2016-07-21T17:11:03Z

std/experimental/allocator/package.d

        }

-        for (; !range.empty; range.popFront, ++i)
+        import std.conv : emplace;


we need the special case for narrow strings & as we already have the special case, using it for arrays will be faster too (see e.g. #4265)

wilzbach · 2016-07-21T17:12:01Z

@9il addressed your comments - you were correct: strings are evil :S
I hope the special-casing I inserted is correct.

@wilzbach please add replaceArrayWithPointer for each function in the module and fix its name in the docs for sliced

Will do this in a separate PR ;-)

9il · 2016-07-22T08:04:23Z

LGTM

9il · 2016-07-22T21:07:02Z

Travis fails, fix/rebase please

schveiguy · 2016-07-22T22:07:07Z

@9il FYI, you can log in your GH user to travis to force a rebuild (as a dlang member). Just figured this out recently. However, in this case, I don't think that will help, as there is an actual issue.

wilzbach · 2016-07-22T23:22:05Z

Travis fails, fix/rebase please

Done. Should be passing now.

9il · 2016-07-23T06:57:40Z

Auto-merge toggled on

9il reviewed Apr 29, 2016
View reviewed changes

wilzbach force-pushed the infer_type_for_make_array_with_ranges branch 3 times, most recently from d099a4e to 6c2b8ca Compare April 29, 2016 15:18

9il reviewed Apr 29, 2016
View reviewed changes

wilzbach force-pushed the infer_type_for_make_array_with_ranges branch from 6c2b8ca to c95b820 Compare April 29, 2016 15:29

wilzbach changed the title ~~infer elementtype for ranges in makeArray,makeSlice~~ infer elementType for ranges in makeArray,makeSlice Apr 29, 2016

9il added @andralex labels May 1, 2016

andralex reviewed Jun 18, 2016
View reviewed changes

andralex removed the @andralex label Jun 18, 2016

wilzbach force-pushed the infer_type_for_make_array_with_ranges branch 2 times, most recently from d61f087 to c44b282 Compare June 19, 2016 21:40

9il reviewed Jun 29, 2016
View reviewed changes

wilzbach force-pushed the infer_type_for_make_array_with_ranges branch 3 times, most recently from 71c87b2 to 7bcd92f Compare July 21, 2016 17:09

wilzbach reviewed Jul 21, 2016
View reviewed changes

wilzbach mentioned this pull request Jul 21, 2016

[ndslice]: rename mod -> replaceWithArray #4634

Merged

infer ElementType for ranges in makeArray and makeSlice

f35efe4

wilzbach force-pushed the infer_type_for_make_array_with_ranges branch from 7bcd92f to f35efe4 Compare July 22, 2016 23:21

9il merged commit 9a00cb5 into dlang:master Jul 23, 2016

wilzbach deleted the infer_type_for_make_array_with_ranges branch August 31, 2016 08:16

Uh oh!

infer elementType for ranges in makeArray,makeSlice #4263

infer elementType for ranges in makeArray,makeSlice #4263

Uh oh!

Conversation

wilzbach commented Apr 29, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

9il Apr 29, 2016

Choose a reason for hiding this comment

Uh oh!

wilzbach Apr 29, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

9il Apr 29, 2016

Choose a reason for hiding this comment

Uh oh!

wilzbach commented Apr 29, 2016

Uh oh!

9il Apr 29, 2016

Choose a reason for hiding this comment

Uh oh!

PetarKirov commented Apr 29, 2016

Uh oh!

wilzbach commented Apr 29, 2016

Uh oh!

MetaLang commented May 3, 2016

Uh oh!

wilzbach commented May 6, 2016

Uh oh!

andralex Jun 18, 2016

Choose a reason for hiding this comment

Uh oh!

wilzbach Jun 19, 2016

Choose a reason for hiding this comment

Uh oh!

PetarKirov Jun 19, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andralex commented Jun 18, 2016

Uh oh!

wilzbach commented Jun 19, 2016

Uh oh!

PetarKirov commented Jun 19, 2016

Uh oh!

JackStouffer commented Jun 29, 2016

Uh oh!

9il Jun 29, 2016

Choose a reason for hiding this comment

Uh oh!

wilzbach Jul 21, 2016

Choose a reason for hiding this comment

Uh oh!

wilzbach commented Jul 21, 2016

Uh oh!

9il commented Jul 22, 2016

Uh oh!

9il commented Jul 22, 2016

Uh oh!

schveiguy commented Jul 22, 2016

Uh oh!

wilzbach commented Jul 22, 2016

Uh oh!

9il commented Jul 23, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

wilzbach commented Apr 29, 2016 •

edited

Loading

wilzbach Apr 29, 2016 •

edited

Loading

PetarKirov Jun 19, 2016 •

edited

Loading