digitalmars.D - std.range's put on arrays needs to change
- WebFreak001 (45/45) Jun 25 2022 let's see this code:
- Basile B. (18/22) Jun 25 2022 I think nobody can disagree. A bit of archeology shows that's
- Paul Backus (14/19) Jun 25 2022 I agree that `put` on arrays does not do what you usually want it
- H. S. Teoh (9/24) Jun 25 2022 Yeah, it's consistent where calling put multiple times writes to
- Paul Backus (3/10) Jun 26 2022 How would you implement .full for, say, an output range that
- Sebastiaan Koppe (2/13) Jun 26 2022 Yes, or a concurrent one.
- Kostiantyn Tokar (12/17) Jun 25 2022 What should I do if I want to preallocate a slice and copy some
- Kostiantyn Tokar (2/6) Jun 26 2022 Should have said "special casing of `putInto` from `put`".
- H. S. Teoh (6/9) Jun 26 2022 Just take a slice of it with [].
- Kostiantyn Tokar (3/11) Jun 26 2022 I'm talking about suggestion. IIUC, with suggested change `put`
- WebFreak001 (7/22) Jun 26 2022 no, the suggestion is only to deprecate the `put` for slices and
- Steven Schveighoffer (3/10) Jun 26 2022 Taking a slice of a slice does nothing.
- Kostiantyn Tokar (21/26) Jun 26 2022 Sorry, I misunderstood. For some reason I thought that `put` is
- Steven Schveighoffer (21/86) Jun 26 2022 It might not be expected, but to disable allowing reference input ranges...
let's see this code: ```d import std.range; int[] arr; arr.put(2); ``` someone not coming from a D backround (or even those coming from it) might expect here to have `arr` be equal to `[2]` afterwards. However this is not the case: ``` core.exception.AssertError /dlang/dmd/linux/bin64/../../src/phobos/std/range primitives.d(2472): Attempting to fetch the front of an empty array of int ---------------- ??:? _d_assert_msg [0x563cbe9c3360] /dlang/dmd/linux/bin64/../../src/phobos/std/range/primitives.d:2472 pure nothrow ref property nogc safe inout(int) std.range.primitives.front!(int).front(return scope inout(int)[]) [0x563cbe9a0fe3] /dlang/dmd/linux/bin64/../../src/phobos/std/range/primitives.d:297 pure nothrow nogc safe void std.range.primitives.doPut!(int[], int).doPut(ref int[], ref int) [0x563cbe9a0f62] /dlang/dmd/linux/bin64/../../src/phobos/std/range/primitives.d:380 pure nothrow nogc safe void std.range.primitives.put!(int[], int).put(ref int[], int) [0x563cbe9a0f37] ./onlineapp.d:8 _Dmain [0x563cbe9a0ee1] ``` this is because `put` is there to write into a pre-allocated buffer if you pass in an array, as [it assumes it is a slice](https://dlang.org/phobos/std_range_primitives.html#put): ```d int[3] buffer; auto slice = buffer[]; put(slice, 1); put(slice, 2); put(slice, 3); assert(!slice.length); assert(buffer == [1,2,3]); ``` which seems to be the way this is intended to be used. However now I argue this is very unintuitive behavior, especially when you compare it to appender!int: ```d auto arr = appender!(int[]); arr.put(1); assert(arr.data == [1]); ``` put on an appender actually adds to the array, the appender allocates memory for it. I strongly feel that this behavior is very inconsistent and unexpected for most users and should be changed. I think we should deprecate simple arrays/slices on `put` and rename the method for those. I propose put with slices should be renamed to `putInto`.
Jun 25 2022
Basile B. <b2.temp gmx.com> On Saturday, 25 June 2022 at 23:44:59 UTC, WebFreak001 wrote:let's see this code: [...] I strongly feel that this behavior is very inconsistent and unexpected for most users and should be changed.I think nobody can disagree. A bit of archeology shows that's it's a very recurring topic: - https://forum.dlang.org/post/pg5jso$s7o$1 digitalmars.com - https://forum.dlang.org/thread/cewdrltvdrojojcxpwyr forum.dlang.org - https://forum.dlang.org/post/sckjdtlsccmfrmgjxpxg forum.dlang.org - https://forum.dlang.org/post/fhnfzlvhxzfxdebciacp forum.dlang.org - https://forum.dlang.org/post/ipg215$25e8$1 digitalmars.com - https://forum.dlang.org/post/h5ensj$1880$1 digitalmars.com - https://forum.dlang.org/thread/h5bdb8$2bcv$1 digitalmars.com - https://issues.dlang.org/show_bug.cgi?id=14998 - https://issues.dlang.org/show_bug.cgi?id=3861 Deprecating the problematic behavior should not be a problem. Since on dynamic arrays `put()` does not do what people think it does, people dont use it.
Jun 25 2022
On Saturday, 25 June 2022 at 23:44:59 UTC, WebFreak001 wrote:I strongly feel that this behavior is very inconsistent and unexpected for most users and should be changed. I think we should deprecate simple arrays/slices on `put` and rename the method for those. I propose put with slices should be renamed to `putInto`.I agree that `put` on arrays does not do what you usually want it to do, but I do not agree that it is inconsistent. It is exactly the same behavior you would get if you used the input range interface and wrote `arr.front = 2; arr.popFront;`. The bigger problem is that unlike an input range's `empty` method, there is no generic way to tell if an output range is full before you try to `put` something into it. And, since `put` fails with an `Error` rather than an `Exception`, you cannot handle failure after the fact either--the only thing you can do is crash. IMO `put` should be changed in `std.v2` to either throw an exception or return `false` when attempting to insert an element into a full output range.
Jun 25 2022
On Sun, Jun 26, 2022 at 04:21:46AM +0000, Paul Backus via Digitalmars-d wrote:On Saturday, 25 June 2022 at 23:44:59 UTC, WebFreak001 wrote:Yeah, it's consistent where calling put multiple times writes to consecutive locations in the original array. [...]I strongly feel that this behavior is very inconsistent and unexpected for most users and should be changed. I think we should deprecate simple arrays/slices on `put` and rename the method for those. I propose put with slices should be renamed to `putInto`.I agree that `put` on arrays does not do what you usually want it to do, but I do not agree that it is inconsistent. It is exactly the same behavior you would get if you used the input range interface and wrote `arr.front = 2; arr.popFront;`.IMO `put` should be changed in `std.v2` to either throw an exception or return `false` when attempting to insert an element into a full output range.Why the needlessly complex workaround? Just introduce a .full method, the natural analogue of .empty for output ranges. T -- Public parking: euphemism for paid parking. -- Flora
Jun 25 2022
On Sunday, 26 June 2022 at 05:10:50 UTC, H. S. Teoh wrote:On Sun, Jun 26, 2022 at 04:21:46AM +0000, Paul Backus via Digitalmars-d wrote:How would you implement .full for, say, an output range that writes to a file?IMO `put` should be changed in `std.v2` to either throw an exception or return `false` when attempting to insert an element into a full output range.Why the needlessly complex workaround? Just introduce a .full method, the natural analogue of .empty for output ranges.
Jun 26 2022
On Sunday, 26 June 2022 at 12:35:10 UTC, Paul Backus wrote:On Sunday, 26 June 2022 at 05:10:50 UTC, H. S. Teoh wrote:Yes, or a concurrent one.On Sun, Jun 26, 2022 at 04:21:46AM +0000, Paul Backus via Digitalmars-d wrote:How would you implement .full for, say, an output range that writes to a file?IMO `put` should be changed in `std.v2` to either throw an exception or return `false` when attempting to insert an element into a full output range.Why the needlessly complex workaround? Just introduce a .full method, the natural analogue of .empty for output ranges.
Jun 26 2022
On Saturday, 25 June 2022 at 23:44:59 UTC, WebFreak001 wrote:I strongly feel that this behavior is very inconsistent and unexpected for most users and should be changed. I think we should deprecate simple arrays/slices on `put` and rename the method for those. I propose put with slices should be renamed to `putInto`.What should I do if I want to preallocate a slice and copy some input range into it? I wouldn't be able to use `std.algorithm.copy` for this simple purpose, because it uses `put` internally. BTW, isn't it a breaking change? It changes semantics of `put` into a slice, and it wouldn't be available in a ` nogc` context. I don't like special casing of slices from all other output ranges. Basically you need to provide analogues for `putInto` of `isOutputRange` and all standard functions that use `put`, like `std.algorithm.copy`, for this sole reason. So why to change this if `std.array.appender` exists?
Jun 25 2022
On Sunday, 26 June 2022 at 06:33:13 UTC, Kostiantyn Tokar wrote:I don't like special casing of slices from all other output ranges. Basically you need to provide analogues for `putInto` of `isOutputRange` and all standard functions that use `put`, like `std.algorithm.copy`, for this sole reason.Should have said "special casing of `putInto` from `put`".
Jun 26 2022
On Sun, Jun 26, 2022 at 06:33:13AM +0000, Kostiantyn Tokar via Digitalmars-d wrote: [...]What should I do if I want to preallocate a slice and copy some input range into it? I wouldn't be able to use `std.algorithm.copy` for this simple purpose, because it uses `put` internally.Just take a slice of it with []. T -- A computer doesn't mind if its programs are put to purposes that don't match their names. -- D. Knuth
Jun 26 2022
On Sunday, 26 June 2022 at 10:29:17 UTC, H. S. Teoh wrote:On Sun, Jun 26, 2022 at 06:33:13AM +0000, Kostiantyn Tokar via Digitalmars-d wrote: [...]I'm talking about suggestion. IIUC, with suggested change `put` will be appending to the slice. And it wouldn't work with ` nogc`.What should I do if I want to preallocate a slice and copy some input range into it? I wouldn't be able to use `std.algorithm.copy` for this simple purpose, because it uses `put` internally.Just take a slice of it with []. T
Jun 26 2022
On Sunday, 26 June 2022 at 10:58:48 UTC, Kostiantyn Tokar wrote:On Sunday, 26 June 2022 at 10:29:17 UTC, H. S. Teoh wrote:no, the suggestion is only to deprecate the `put` for slices and disallow it completely with that function name, to avoid errors in code bases. Appending to the slice would not work because of the ` nogc` change and also because it's a backwards-incompatible breaking change.On Sun, Jun 26, 2022 at 06:33:13AM +0000, Kostiantyn Tokar via Digitalmars-d wrote: [...]I'm talking about suggestion. IIUC, with suggested change `put` will be appending to the slice. And it wouldn't work with ` nogc`.What should I do if I want to preallocate a slice and copy some input range into it? I wouldn't be able to use `std.algorithm.copy` for this simple purpose, because it uses `put` internally.Just take a slice of it with []. T
Jun 26 2022
On 6/26/22 6:29 AM, H. S. Teoh wrote:On Sun, Jun 26, 2022 at 06:33:13AM +0000, Kostiantyn Tokar via Digitalmars-d wrote: [...]Taking a slice of a slice does nothing. -SteveWhat should I do if I want to preallocate a slice and copy some input range into it? I wouldn't be able to use `std.algorithm.copy` for this simple purpose, because it uses `put` internally.Just take a slice of it with [].
Jun 26 2022
Sorry, I misunderstood. For some reason I thought that `put` is suggested to append to a slice, like `put` with `appender`. But anyway, I still want to ask the question about functions that use `put` internally. On Saturday, 25 June 2022 at 23:44:59 UTC, WebFreak001 wrote:I strongly feel that this behavior is very inconsistent and unexpected for most users and should be changed. I think we should deprecate simple arrays/slices on `put` and rename the method for those. I propose put with slices should be renamed to `putInto`.To put it into a context, consider the following code: ``` void main() nogc { import std : iota, copy, save; import core.stdc.stdlib : malloc; auto n = 5; auto buffer = (cast(int*) malloc(n * int.sizeof))[0..n]; n.iota.copy(buffer.save); } ``` Currently this code fills the buffer, allocated by `malloc`, with the elements of `iota`. Now, assuming the suggested change is implemented, how do I replicate the current behaviour after the deprecation period? Iterate over `iota` and call `putInto` directly?
Jun 26 2022
On 6/25/22 7:44 PM, WebFreak001 wrote:let's see this code: ```d import std.range; int[] arr; arr.put(2); ``` someone not coming from a D backround (or even those coming from it) might expect here to have `arr` be equal to `[2]` afterwards. However this is not the case: ``` core.exception.AssertError /dlang/dmd/linux/bin64/../../src/phobos/std/range primitives.d(2472): Attempting to fetch the front of an empty array of int ---------------- ??:? _d_assert_msg [0x563cbe9c3360] /dlang/dmd/linux/bin64/../../src/phobos/std/range/primitives.d:2472 pure nothrow ref property nogc safe inout(int) std.range.primitives.front!(int).front(return scope inout(int)[]) [0x563cbe9a0fe3] /dlang/dmd/linux/bin64/../../src/phobos/std/range/primitives.d:297 pure nothrow nogc safe void std.range.primitives.doPut!(int[], int).doPut(ref int[], ref int) [0x563cbe9a0f62] /dlang/dmd/linux/bin64/../../src/phobos/std/range/primitives.d:380 pure nothrow nogc safe void std.range.primitives.put!(int[], int).put(ref int[], int) [0x563cbe9a0f37] ./onlineapp.d:8 _Dmain [0x563cbe9a0ee1] ``` this is because `put` is there to write into a pre-allocated buffer if you pass in an array, as [it assumes it is a slice](https://dlang.org/phobos/std_range_primitives.html#put): ```d int[3] buffer; auto slice = buffer[]; put(slice, 1); put(slice, 2); put(slice, 3); assert(!slice.length); assert(buffer == [1,2,3]); ``` which seems to be the way this is intended to be used. However now I argue this is very unintuitive behavior, especially when you compare it to appender!int: ```d auto arr = appender!(int[]); arr.put(1); assert(arr.data == [1]); ``` put on an appender actually adds to the array, the appender allocates memory for it. I strongly feel that this behavior is very inconsistent and unexpected for most users and should be changed. I think we should deprecate simple arrays/slices on `put` and rename the method for those. I propose put with slices should be renamed to `putInto`.It might not be expected, but to disable allowing reference input ranges as output ranges would break so many fundamental usages, such as `std.algorithm.copy`. You wouldn't just be disabling slices, it would be a complete removal of a whole class of output ranges. What we need is an appendTo wrapper: ```d struct AppendTo(T) { private T[] *arr; this(return T[]* arr) { this.arr = arr; } void put(X)(X val) if (__traits(compiles, (*arr) ~= val)) { (*arr) ~= val; } } auto appendTo(T)(return ref T[] arr) { return AppendTo!T(&arr); } ``` -Steve
Jun 26 2022