Bug 15982 – std.array.array treats dynamic arrays as input ranges and allocates new memory
Status
RESOLVED
Resolution
WONTFIX
Severity
enhancement
Priority
P1
Component
phobos
Product
D
Version
D2
Platform
All
OS
All
Creation time
2016-05-02T07:55:00Z
Last change time
2017-06-27T16:28:22Z
Assigned to
nobody
Creator
sigod.mail
Comments
Comment #0 by sigod.mail — 2016-05-02T07:55:40Z
import std.array : array;
int[] a = [1, 2, 3];
assert(a.ptr == array(a).ptr); // fails
I think for dynamic arrays `array()` should just return provided value without doing anything.
Comment #1 by ag0aep6g — 2016-05-02T10:59:43Z
This would be a serious breaking change. The documentation says that the function allocates and copies.
Comment #2 by sigod.mail — 2016-05-02T11:06:15Z
It's meaningless for dynamic arrays.
Currently, if you change you code from ranges to arrays you have to remove use of `array()` or it just adds hidden cost.
Comment #3 by ag0aep6g — 2016-05-02T11:24:17Z
(In reply to sigod from comment #2)
> It's meaningless for dynamic arrays.
Currently std.array.array guarantees one level of duplication. So when I call it on an int[], I can rely on the new array being independent from the original one. I can alter elements without affecting the original. I can cast it to immutable(int)[] without running into undefined behavior when the original is altered.
I'm not saying that this is the best behavior for a function called "array", but that's how it's documented and how it works. Changing it now would be a serious breaking change.
Comment #4 by sigod.mail — 2016-05-02T11:39:39Z
(In reply to ag0aep6g from comment #3)
> (In reply to sigod from comment #2)
> > It's meaningless for dynamic arrays.
>
> Currently std.array.array guarantees one level of duplication. So when I
> call it on an int[], I can rely on the new array being independent from the
> original one. I can alter elements without affecting the original. I can
> cast it to immutable(int)[] without running into undefined behavior when the
> original is altered.
One of the first things that I learned in D is that you have `dup` and `idup` "properties" for this. I bet no one uses `array()` for duplicating arrays.
> I'm not saying that this is the best behavior for a function called "array",
> but that's how it's documented and how it works. Changing it now would be a
> serious breaking change.
Function called `array` for a reason. Because it gives you array where you have only an input range. It doesn't make sense to do anything if you already have an array. In normal code you will never call `array` for array.
Comment #5 by ag0aep6g — 2016-05-02T12:06:54Z
(In reply to sigod from comment #4)
> I bet no one uses `array()` for duplicating
> arrays.
I don't think betting on these things is a good course of action. The function is documented to allocate a new array. Changing that is a breaking change.
> Function called `array` for a reason. Because it gives you array where you
> have only an input range. It doesn't make sense to do anything if you
> already have an array. In normal code you will never call `array` for array.
Here's a little generic function that relies on std.array.array's current behavior:
----
immutable(ElementType!R)[] toImmutableArray(R)(R range)
if (isInputRange!R && !hasIndirections!(ElementType!R))
{
import std.array: array;
import std.exception: assumeUnique;
return assumeUnique(range.array);
}
----
Comment #6 by ag0aep6g — 2016-05-02T12:08:24Z
(In reply to ag0aep6g from comment #5)
> Here's a little generic function that relies on std.array.array's current
> behavior:
> ----
> immutable(ElementType!R)[] toImmutableArray(R)(R range)
> if (isInputRange!R && !hasIndirections!(ElementType!R))
> {
> import std.array: array;
> import std.exception: assumeUnique;
> return assumeUnique(range.array);
> }
> ----
Missing imports:
----
import std.range: ElementType, isInputRange;
import std.traits: hasIndirections;
----
Comment #7 by jack — 2016-05-02T17:48:41Z
(In reply to sigod from comment #4)
> I bet no one uses `array()` for duplicating
> arrays.
Maybe, maybe not. But I'd bet $500 that people rely on the functionality and don't know it. If this is changed you'll have people suddenly wondering why their arrays are getting filled garbage data.
With a function as popular as std.array.array, I'd say this change is way too dangerous.
Comment #8 by sigod.mail — 2016-05-02T18:43:09Z
I'm not betting on anything. It was a figure of speech.
Comment #9 by jack — 2016-05-02T18:58:07Z
(In reply to sigod from comment #8)
> I'm not betting on anything. It was a figure of speech.
The best way to call someone's bluff is to put money on the table. If you were sure that no one was using this for duplication, then you would have taken my bet.
Comment #10 by sigod.mail — 2016-05-02T19:11:06Z
(In reply to Jack Stouffer from comment #9)
> (In reply to sigod from comment #8)
> > I'm not betting on anything. It was a figure of speech.
>
> The best way to call someone's bluff is to put money on the table. If you
> were sure that no one was using this for duplication, then you would have
> taken my bet.
I don't gamble. Especially when it's impossible to win.
(Let's end off-topic here.)
Comment #11 by dlang-bugzilla — 2017-06-27T16:28:22Z
As discussed above, std.array.array currently guarantees that the data it returns is unique. This guarantee in turn allows some assumptions, such as that writing to the result will not have side effects, that it is safe to pass to other parts of the program, or even delete it. I suppose that if the array were to point to immutable elements, avoiding reallocation might be worth considering; though, currently, even .idup will duplicate an immutable array.
As it is, this issue is missing a use case, and it will probably need to be a compelling one to warrant changing the function's contract and risking code breakage. Other than that... well, if you don't want your array reallocated, then just don't call array()?
Please reopen if you have a good argument why this should be changed despite the above.