http://d.puremagic.com/issues/show_bug.cgi?id=11082

           Summary: std.algorithm.join of a dynamic array of fixed-size
                    arrays
           Product: D
           Version: D2
          Platform: All
        OS/Version: All
            Status: NEW
          Keywords: rejects-valid
          Severity: enhancement
          Priority: P2
         Component: Phobos
        AssignedTo: nobody@puremagic.com
        ReportedBy: bearophile_hugs@eml.cc


--- Comment #0 from bearophile_hugs@eml.cc 2013-09-21 05:30:56 PDT ---
import std.algorithm: join;
void main() {
    int[2][] a = [[1, 2]];
    join(a);
}



test.d(4): Error: template std.array.join does not match any function template
declaration. Candidates are:
...\dmd2\src\phobos\std\array.d(1449):        std.array.join(RoR, R)(RoR ror, R
sep) if (isInputRange!RoR && isInputRange!(ElementType!RoR) && isInputRange!R
&& is(Unqual!(ElementType!(ElementType!RoR)) == Unqual!(ElementType!R)))
...\dmd2\src\phobos\std\array.d(1496):        std.array.join(RoR)(RoR ror) if
(isInputRange!RoR && isInputRange!(ElementType!RoR))
test.d(4): Error: template std.array.join(RoR, R)(RoR ror, R sep) if
(isInputRange!RoR && isInputRange!(ElementType!RoR) && isInputRange!R &&
is(Unqual!(ElementType!(ElementType!RoR)) == Unqual!(ElementType!R))) cannot
deduce template function from argument types !()(int[2][])


See also Issue 7690

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=11082


Peter Alexander <peter.alexander.au@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |peter.alexander.au@gmail.co
                   |                            |m


--- Comment #1 from Peter Alexander <peter.alexander.au@gmail.com> 2013-09-21 05:54:01 PDT ---
join works on a range of ranges, but here you have a range of static arrays, which are not ranges.

I'm not sure if this is a valid enhancement. I'm not a fan of making special allowances for static arrays. If you had an array of containers it wouldn't work either.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=11082



--- Comment #2 from bearophile_hugs@eml.cc 2013-09-21 06:14:46 PDT ---
(In reply to comment #1)

> join works on a range of ranges, but here you have a range of static arrays, which are not ranges.

I don't care how you call those things inside the outer array, I call them "fixed sized arrays", they are language built-ins, and sometimes I have had to collect them in a larger dynamic array. I have written a join function to join them in my own code, and I think such functionality should be present in Phobos. I see no good reason to keep such so basic functionality out of Phobos and inside my own libraries.

Workarounds like this one are a waste of efficiency and they are noisy:

import std.algorithm: join;
import std.stdio, std.algorithm;
void main() {
    int[2][] data = [[1, 2]];
    data.map!q{ a[] }.join.writeln;
}


Also, that code doesn't work, you could avoid that trap with a dup:

import std.algorithm: join;
import std.stdio, std.algorithm;
void main() {
    int[2][] data = [[1, 2]];
    data.map!(a => a.dup).join.writeln;
}



> I'm not sure if this is a valid enhancement. I'm not a fan of making special allowances for static arrays.

C++, Ada and Rust languages show that if you want an efficient system language you should encourage and help programmers avoid dynamic allocations where possible. Fixed sized arrays help reduce heap allocations, and they should be supported as much as possible by Phobos, instead of making them second-class citizens. It's not a problem of those arrays, it's a problem of the Range abstraction.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=11082



--- Comment #3 from Peter Alexander <peter.alexander.au@gmail.com> 2013-09-21 06:37:47 PDT ---
(In reply to comment #2)
> (In reply to comment #1)
> > I'm not sure if this is a valid enhancement. I'm not a fan of making special allowances for static arrays.
> 
> C++, Ada and Rust languages show that if you want an efficient system language you should encourage and help programmers avoid dynamic allocations where possible. Fixed sized arrays help reduce heap allocations, and they should be supported as much as possible by Phobos, instead of making them second-class citizens. It's not a problem of those arrays, it's a problem of the Range abstraction.

I agree, and I use static arrays all the time, so I understand your frustrations. However, they are not second class citizens, at least not anymore than Array, SList, or any other container. The issue is that dynamic arrays are more than first-class citizens, they are a freak type that is strange combination of container and range.

I do not look forward to having to write special overloads for every range-operating function that also accepts containers. It is impractical and a maintenance nightmare. There needs to be a better solution, so I would suggest we either find a way of automating this transform, or just live with the minor inconvenience of working with containers instead of ranges.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=11082


Jonathan M Davis <jmdavisProg@gmx.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jmdavisProg@gmx.com


--- Comment #4 from Jonathan M Davis <jmdavisProg@gmx.com> 2013-09-21 10:48:34 PDT ---
We're just asking for trouble if we try and treat containers as ranges. We have enough trouble with dynamic arrays, and they're only pseudo-containers.

static arrays are _not_ ranges, should not be treated as such, and _cannot_ be treated as such. You'd have to special case algorithms to handle them, because they violate the range API by their very nature. And it's trivial enough to slice static arrays - even when they're inside a dynamic array - that I really don't think that it's worth complicating things with special cases for static arrays. I think that it's bad enough that static arrays are implicitly sliced when passed to a function which takes a dynamic array, as that's inherently unsafe. What you're asking for just makes that problem worse.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=11082



--- Comment #5 from bearophile_hugs@eml.cc 2013-09-21 11:30:06 PDT ---
(In reply to comment #4)

> You'd have to special case algorithms to handle them, because they violate the range API by their very nature.

Right, I am asking for a specialization of join. It's worth doing.


> And it's trivial enough to
> slice static arrays - even when they're inside a dynamic array -

This code is buggy:

import std.algorithm: join;
import std.stdio, std.algorithm;
void main() {
    int[2][] data = [[1, 2]];
    data.map!q{ a[] }.join.writeln;
}

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=11082



--- Comment #6 from Peter Alexander <peter.alexander.au@gmail.com> 2013-09-21 11:36:14 PDT ---
(In reply to comment #5)
> (In reply to comment #4)
> 
> > You'd have to special case algorithms to handle them, because they violate the range API by their very nature.
> 
> Right, I am asking for a specialization of join. It's worth doing.

But is it worth doing for every other algorithm, and every container type? If not, why not? If so, I'd say that's too much to ask.


> > And it's trivial enough to
> > slice static arrays - even when they're inside a dynamic array -
> 
> This code is buggy:
> 
> import std.algorithm: join;
> import std.stdio, std.algorithm;
> void main() {
>     int[2][] data = [[1, 2]];
>     data.map!q{ a[] }.join.writeln;
> }

You have to do:

data.map!((ref a => a[])).join.writeln;

Unfortunately shorthand lambdas and unaryFun default to pass-by-value, so you get a stack copy of your static array, which you then slice just as it goes out of scope.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=11082



--- Comment #7 from bearophile_hugs@eml.cc 2013-09-21 13:18:09 PDT ---
(In reply to comment #6)

> But is it worth doing for every other algorithm, and every container type?

Probably not. But I think it's worth doing for a small subset of algorithms, as std.algorithm.join.

Regarding containers, fixed sized arrays are built-ins, and they are supposed to be used often, so it's not unreasonable to handle them differently/better than library-defined containers.


> You have to do:
> 
> data.map!((ref a => a[])).join.writeln;
> 
> Unfortunately shorthand lambdas and unaryFun default to pass-by-value, so you get a stack copy of your static array, which you then slice just as it goes out of scope.

DMD has to give a compile-time error for such situation. I think there's an enhancement request opened on that.

Joining a sequence of fixed-size arrays is a commonly done operation (in my code), and that map+join code is bug-prone. Your code too has a small mistake:


import std.algorithm: join;
import std.stdio, std.algorithm;
void main() {
    int[2][] data = [[1, 2]];
    data.map!((ref a) => a[]).join.writeln;
}

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------