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

           Summary: std.algorithm.map!(T) cause CT error for fixed size
                    arrays
           Product: D
           Version: D2
          Platform: x86
        OS/Version: Windows
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Phobos
        AssignedTo: nobody@puremagic.com
        ReportedBy: marmyst@gmail.com


--- Comment #0 from Marcin Mstowski <marmyst@gmail.com> 2013-02-24 06:43:45 PST ---
void main() {
  float[5] smt = [0, 1, 2, 4, 100];
  auto ms = map!"a*a"(smt);
}

DMD 2.062 gives:

dmd test.d
test.d(5): Error: template std.algorithm.map!("a*a").map does not match any
function template declaration. Candidates are:
..\..\src\phobos\std\algorithm.d(379):
std.algorithm.map!("a*a").map(Range)(Range r) if
(isInputRange!(Unqual!(Range)))
test.d(5): Error: template std.algorithm.map!("a*a").map(Range)(Range r) if
(isInputRange!(Unqual!(Range))) cannot deduce template function from argument
types!()(float[5u])

Dynamic arrays for map!T and reduce!T with static arrays (not tested for others functions) compile without problem.

-- 
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=9582


bearophile_hugs@eml.cc changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bearophile_hugs@eml.cc


--- Comment #1 from bearophile_hugs@eml.cc 2013-02-24 07:31:48 PST ---
(In reply to comment #0)

> void main() {
>   float[5] smt = [0, 1, 2, 4, 100];
>   auto ms = map!"a*a"(smt);
> }

Only certain higher order functions of Phobos work with fixed-sized arrays:


import std.algorithm: reduce;
void main() {
    float[5] smt = [0, 1, 2, 4, 100];
    assert(reduce!"a+b"(smt) == 107); // OK.
}


So probably this bug report should be closed.

-- 
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=9582


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

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


--- Comment #2 from Peter Alexander <peter.alexander.au@gmail.com> 2013-02-24 08:08:11 PST ---
This is by design. map expects an input range. Static arrays are not input ranges. You have to take a slice.

auto ms = map!"a*a"(smt[]); // note additional []

reduce only requires an iterable, which includes static arrays.

-- 
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=9582


monarchdodra@gmail.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |monarchdodra@gmail.com


--- Comment #3 from monarchdodra@gmail.com 2013-02-24 08:11:52 PST ---
(In reply to comment #2)
> This is by design. map expects an input range. Static arrays are not input ranges. You have to take a slice.
> 
> auto ms = map!"a*a"(smt[]); // note additional []
> 
> reduce only requires an iterable, which includes static arrays.

...and this is good example of why accepting an "iterable" as a function argument, as opposed to a range, is usually a bad idea.

As for reduce accepting iterable, Andrei asked me to remove it in one of my open requests, so don't expect it keep working for too long.

https://github.com/D-Programming-Language/phobos/pull/861#discussion_r2024457

-- 
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=9582



--- Comment #4 from Peter Alexander <peter.alexander.au@gmail.com> 2013-02-24 08:16:44 PST ---
(In reply to comment #3)
> ...and this is good example of why accepting an "iterable" as a function argument, as opposed to a range, is usually a bad idea.
> 
> As for reduce accepting iterable, Andrei asked me to remove it in one of my open requests, so don't expect it keep working for too long.
> 
> https://github.com/D-Programming-Language/phobos/pull/861#discussion_r2024457

std.array.array also accepts iterables. I think array and reduce are the only two.

Removing these is going to break a lot of code though.

-- 
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=9582



--- Comment #5 from monarchdodra@gmail.com 2013-02-24 08:21:49 PST ---
(In reply to comment #4)
> (In reply to comment #3)
> > ...and this is good example of why accepting an "iterable" as a function argument, as opposed to a range, is usually a bad idea.
> > 
> > As for reduce accepting iterable, Andrei asked me to remove it in one of my open requests, so don't expect it keep working for too long.
> > 
> > https://github.com/D-Programming-Language/phobos/pull/861#discussion_r2024457
> 
> std.array.array also accepts iterables. I think array and reduce are the only two.
> 
> Removing these is going to break a lot of code though.

For array, probably. For reduce, probably much less.

I think ideally, we should accept "isNonStaticArrayItterable":

The idea of "isIterable" is to accept stuff that is either a range, or has opApply. Static arrays, while being iterable, shouldn't be passed around by value to functions.

I think there'd be gains in the long run to such a scheme.

-- 
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=9582



--- Comment #6 from bearophile_hugs@eml.cc 2013-02-24 08:52:34 PST ---
(In reply to comment #3)

> As for reduce accepting iterable, Andrei asked me to remove it in one of my open requests, so don't expect it keep working for too long.

I am strongly against this breaking change.

-- 
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=9582



--- Comment #7 from bearophile_hugs@eml.cc 2013-02-24 13:49:05 PST ---
(In reply to comment #6)

> I am strongly against this breaking change.

Sorry, that came out too much strong...

-- 
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=9582



--- Comment #8 from monarchdodra@gmail.com 2013-02-24 13:59:12 PST ---
(In reply to comment #7)
> (In reply to comment #6)
> 
> > I am strongly against this breaking change.
> 
> Sorry, that came out too much strong...

Don't worry about it, I didn't even perceive anything that could be offensive.

Can you elaborate on the "why" though? Is it just because it would be "breaking change", or do you have a use case that warrants its being in there in the first place (more so than any algorithm I mean)?

-- 
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=9582



--- Comment #9 from bearophile_hugs@eml.cc 2013-02-24 14:40:31 PST ---
(In reply to comment #8)

> Can you elaborate on the "why" though? Is it just because it would be "breaking change", or do you have a use case that warrants its being in there in the first place (more so than any algorithm I mean)?

I have a good amount of functional-style D2 code with hundreds of usages of reduce. And in D I use fixed-sized arrays all the time to reduce heap activity, so probably this change will require me some fixes in my code, to add the [].

I don't like this change in principle, because I like fixed-size arrays, I use them all the time, and I think they should be more first class in Phobos. I'd like more Phobos functions to work with fixed size arrays, not less functions.

Also, reduce() doesn't return part of the input, it creates a new output, so it
the code that uses the result of reduce() doesn't need to care what reduce()
was fed with. The same is true for array(). I think reduce() and array() should
also work on opApply, for maximum flexibility.

The higher order functions of my D1 nonstandard library accepted both kinds of arrays, and even associative arrays as inputs. I prefer more flexibility. And I think that forcing everything inside the mindframe of Ranges is a not a good idea. Iteration life is more complex: http://journal.stuffwithstuff.com/2013/01/13/iteration-inside-and-out/

In past I have asked one small changes in Phobos that was closed down because despite it being handy and useful, it was a small breaking change. Forbidding fixed size arrays as reduce inputs _reduces_ flexibility, and it's a larger breaking change.

In the end I am probably able to add the missing [] to my D2 code in a matter of one or two hours (or less) so for me this change doesn't require me a lot of work to fix. So I leave the decision to you.

---------------

> Static arrays, while being iterable, shouldn't be passed around by
value to functions. I think there'd be gains in the long run to such a scheme.<

Let's say I have this fixed-sized array:

int[3] items = [1, 2, 3];

and I want to compute its sum:

reduce!q{a + b}(items)

In this case ideally I'd like the D compiler to replace that call to reduce with the same ASM as

items[0] + items[1] + items[2]

This means the compiler has some compile-time information (the array length) that in theory (and in practice too if the code is well written and you are using a GCC back-end that is able to unroll small loops) is usable to produce full optimized code.

If I have to write:

reduce!q{a + b}(items[])

Then we have lost something, the reduce() can't know at compile-time the length of the array, so performing loop unrolling becomes harder (the JavaVM is able to partially unroll the loop in this case too, but LLVM was not able to do it since the latest version, and even now it's not perfect).

Throwing away some compile-time information seems a bad idea to me.

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