Bug 8910 – Static arrays, dynamic arrays and std.array.join

I imagine this bug is a mere matter of adjusting some templates, but it is a very common occurence anyway. The simplest case: char[5][2] cs; cs.join(","); // fails The same case, rewritten to use heap arrays. char[][] cs = new char[][2]; cs[0] = new char[5]; cs[1] = new char[5]; cs.join(","); // works

The error is "cannot deduce template function from argument types !()(char[2LU][], char[])

The problem is that you're using static arrays, and they're not ranges. They will work with some range-based functions if you slice them (as their slice is a dynamic array), but static arrays themselves will not work. But you need to be careful when slicing them and passing the slices to range-based functions, because you're then slicing memory which is on the stack, and if it escapes the function, then it'll be pointing at invalid memory (since the static array won't exist anymore).

This too fails: void main() { char[5][2] cs; cs[].join(","); } This is a work-around, but I agree it's too much complex and it produces too much garbage for the collector (all those dup): import std.stdio, std.array, std.algorithm; void main() { char[5][2] cs = 'x'; char[] j = map!(r => r.dup)(cs[]).join(","); writeln(j); } So I agree join() should support fixed-size arrays of arrays.

(In reply to comment #2) > The problem is that you're using static arrays, and they're not ranges. They > will work with some range-based functions if you slice them (as their slice is > a dynamic array), but static arrays themselves will not work. But you need to > be careful when slicing them and passing the slices to range-based functions, > because you're then slicing memory which is on the stack, and if it escapes the > function, then it'll be pointing at invalid memory (since the static array > won't exist anymore). What you say doesn't explain why Phobos doesn't have a function to join a built-in fixed-sized 2D array turning it into a single dynamic array. So I think closing this issue is the wrong decision.

This works without allocating: import std.stdio, std.array, std.algorithm; void main() { char[5][2] cs = 'x'; char[] j = cs[].map!((char[]a)=>a).join(","); writeln(j); }

(In reply to comment #5) > char[] j = cs[].map!((char[]a)=>a).join(","); Right, good idea. It also avoids all the stack copies of the rows! :-)

> What you say doesn't explain why Phobos doesn't have a function to join a > built-in fixed-sized 2D array turning it into a single dynamic array. So I > think closing this issue is the wrong decision. That would mean copying it. The solution is to slice it, and Timon's solution deals with the multi-dimensional issues posed by this particular case.

Frankly thats a terrible alternative. Perhaps static array specialisations are in order then; assuming current implementations can't be modified to suit.

> Frankly thats a terrible alternative. Perhaps static array specialisations are > in order then; assuming current implementations can't be modified to suit. So, you think that copying a static array is a good idea? Because that's what on overload for a static array would do. And all that overload could do would be to slice the static array and pass it to the normal overload (because static arrays are _not_ ranges and _cannot_ be, because you can't pop any of their elements off), which would lead to the slice being completely invalid once the function returned, meaning that the result would be completely unsafe and invalid. So no, that wouldn't work.

(In reply to comment #9) > > Frankly thats a terrible alternative. Perhaps static array specialisations are > > in order then; assuming current implementations can't be modified to suit. > > So, you think that copying a static array is a good idea? Because that's what > on overload for a static array would do. And all that overload could do would > be to slice the static array and pass it to the normal overload (because static > arrays are _not_ ranges and _cannot_ be, because you can't pop any of their > elements off), which would lead to the slice being completely invalid once the > function returned, meaning that the result would be completely unsafe and > invalid. So no, that wouldn't work. I do think it is a good idea, no, but that was the "clearest" solution... second to that proposed by Timon: map!(c => c[])(cs[]).join(","); But if what you say is set in stone, then there is no point taking it further. Maybe other than the fact putting something in the documentation/compiler warning so that the error message that is given is not so confusing to those coming from other languages.

(In reply to comment #9) > > Frankly thats a terrible alternative. Perhaps static array specialisations are > > in order then; assuming current implementations can't be modified to suit. > > So, you think that copying a static array is a good idea? Because that's what > on overload for a static array would do. And all that overload could do would > be to slice the static array and pass it to the normal overload (because static > arrays are _not_ ranges and _cannot_ be, because you can't pop any of their > elements off), which would lead to the slice being completely invalid once the > function returned, meaning that the result would be completely unsafe and > invalid. So no, that wouldn't work. I do not think it is a good idea, no, but that was the "clearest" solution... second to that proposed by Timon. map!(c => c[])(cs[]).join(","); But if what you say is unchanging, then there is no point taking it further. Maybe putting something in the documentation/compiler warning so that the error message that is given is not so confusing to those coming from other languages could help.

Sigh, ignore comment 10... (or delete it, and this)

(In reply to comment #9) > > Frankly thats a terrible alternative. Perhaps static array specialisations are > > in order then; assuming current implementations can't be modified to suit. > > So, you think that copying a static array is a good idea? Because that's what > on overload for a static array would do. ... auto rangeFunction(T)(ref T range) if(isStaticArray!T && ...){ return rangeFunction(range[]); } I do not think implicitly slicing static arrays for all range-based functions is really worth the trouble. It would silence all those fruitless discussions though.

(In reply to comment #11) > ... > map!(c => c[])(cs[]).join(","); > ... This is not valid.

(In reply to comment #14) > (In reply to comment #11) > > ... > > map!(c => c[])(cs[]).join(","); > > ... > > This is not valid. Huh. So it isn't, it compiles however, just garbage output. What *is* it doing differently? Also, thanks for bearing with me on this Jonathan M Davis, you're explanation has been a great helping in understanding the problem. :)

(In reply to comment #9) > So, you think that copying a static array is a good idea? Because that's what > on overload for a static array would do. And all that overload could do would > be to slice the static array and pass it to the normal overload (because static > arrays are _not_ ranges and _cannot_ be, because you can't pop any of their > elements off), which would lead to the slice being completely invalid once the > function returned, meaning that the result would be completely unsafe and > invalid. So no, that wouldn't work. This bug report is about a join() that works with fixed-size matrix. join() is supposed to allocate a new array as large as the sum of the rows plus the optional separators. And if this join function is well written (taking fixed sized array by reference and not using the dynamic-array function badly as you say), the result is correct and the requested computation is done efficiently, it's more efficient than the case with dynamic arrays, because there is less indirection and being all the rows of the same length there is no need to read and use every length of each row as in the dynamic array case, because it's surely rectangular.

(In reply to comment #15) > (In reply to comment #14) > > (In reply to comment #11) > > > ... > > > map!(c => c[])(cs[]).join(","); > > > ... > > > > This is not valid. > > Huh. So it isn't, it compiles however, just garbage output. > What *is* it doing differently? > > Also, thanks for bearing with me on this Jonathan M Davis, you're explanation > has been a great helping in understanding the problem. :) Right, so it is passing in a random static array of char[5] into the function, which contains garbage. By doing `char[] j = map!((char[] c) => c)(cs[]).join(",");`, you are forcing the template input to be a char[], which has implicit casting, and therefore returns a slice (note, AFAIK the second slice wasn't necessary on c).

(In reply to comment #15) > (In reply to comment #14) > > (In reply to comment #11) > > > ... > > > map!(c => c[])(cs[]).join(","); > > > ... > > > > This is not valid. > > Huh. So it isn't, it compiles however, just garbage output. > What *is* it doing differently? > ... This copies the static arrays to a function-local parameter, slices them and returns those slices. i.e. it escapes stack addresses. It should be a compile error. Mine implicitly slices the original static arrays before they are passed in and returns those slices.

Slicing static arrays is unsafe (and really should be considered to be @system - issue# 8838), so it really shouldn't be happening silently. If it were up to me, static arrays would _never_ be implicitly sliced precisely because of how dangerous it is. It's useful and should be permitted, but it should be explicit.

(In reply to comment #19) > Slicing static arrays is unsafe That's true because the type system of D is primitive. In a little more refined type system, this is not true. > If it were up to > me, static arrays would _never_ be implicitly sliced precisely because of how > dangerous it is. It's useful and should be permitted, but it should be > explicit. But this is unrelated to this enhancement request, because it's easy to write a join() on fixed-sized 2D matrix that is safe.