https://issues.dlang.org/show_bug.cgi?id=12583

Steven Schveighoffer <schveiguy@yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |schveiguy@yahoo.com

--- Comment #1 from Steven Schveighoffer <schveiguy@yahoo.com> ---
Fully agree with this.

In fact, retro could accept any type that supports .retro(), even non-ranges.

I wonder if foreach_reverse could be specialized to call this on bidirectional ranges...

Note, this is a similar pattern to 'put', I wonder if there is a commonality here we can extract into a "D design pattern".

--

https://issues.dlang.org/show_bug.cgi?id=12583

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

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

--- Comment #2 from Jonathan M Davis <jmdavisProg@gmx.com> ---
I think that having std.range.retro call the member function retro if there is one is fine, but I should point out that this is a general problem which is not at all specific to retro. Pretty much _any_ free function can be replaced with a member function which is more efficient if a user-defined type has a way of doing the same thing more efficiently. So, to solve this problem in the general case, I think we have to do one of two things:

1. Make it standard policy to have a free function check if there is a member function which takes the same arguments and then calls that instead of using its own implementation if such a function exists.

2. Make it standard policy to always use UFCS, in which case, the member function will always be used if it exists.

In general, I would argue simply for going with #2, because it's less of a maintenance nightmare, though doing #1 in addition to #2 would catch the cases when someone fails to do #2. Still, I really think that this problem is simply an argument for why UFCS should always be used where possible. And personally, I think that this problem is really the only reason that UFCS is worth having. All of the other reasons for it that I can think of are purely subjective, where in this case, it actually solves a real problem to use it.

--

https://issues.dlang.org/show_bug.cgi?id=12583

--- Comment #3 from monarchdodra@gmail.com ---
(In reply to Jonathan M Davis from comment #2)
> I think that having std.range.retro call the member function retro if there is one is fine, but I should point out that this is a general problem which is not at all specific to retro. Pretty much _any_ free function can be replaced with a member function which is more efficient if a user-defined type has a way of doing the same thing more efficiently. So, to solve this problem in the general case, I think we have to do one of two things:
> 
> 1. Make it standard policy to have a free function check if there is a member function which takes the same arguments and then calls that instead of using its own implementation if such a function exists.
> 
> 2. Make it standard policy to always use UFCS, in which case, the member function will always be used if it exists.

The issue is that 2 can clash with 1, if the type in question has the member function, but not with the correct args. A typical example is `put`: It should *not* be used UFCS:

//----
struct S
{
    void put(int){};
}

void main()
{
    S s;
    int[] a;
    put(s, a); //Fine, put the array a into s.
    s.put(a); //Nope, don't know how to.
}
//----

So, if (for some strange reason), I were to add "retro(int)" to my function,
but not "retro()", then a UFCS call to "myRange.retro()" would fail.

--

https://issues.dlang.org/show_bug.cgi?id=12583

--- Comment #4 from Jonathan M Davis <jmdavisProg@gmx.com> ---
If myRange.retro() fails, because typeof(myRange) defined retro(int) but not
retro, then that sounds like a bug with UFCS to me. retro(int) doesn't match
myRange.retro(), but std.range.retro does, so std.range.retro should be called
rather than the member function.

If a member function has a different signature than the free function, then it's not intended as a replacement for the free function, and it shouldn't be conflicting with the free function at all.

--

https://issues.dlang.org/show_bug.cgi?id=12583

--- Comment #5 from Steven Schveighoffer <schveiguy@yahoo.com> ---
(In reply to Jonathan M Davis from comment #4)

> If a member function has a different signature than the free function, then it's not intended as a replacement for the free function, and it shouldn't be conflicting with the free function at all.

No, it's not a bug. Lookup rules work on overload sets, not on individual functions. If you define a ufcs method that overloads with a straight method, you cannot call the ufcs method, this is intended.

--

https://issues.dlang.org/show_bug.cgi?id=12583

--- Comment #6 from Jonathan M Davis <jmdavisProg@gmx.com> ---
> No, it's not a bug. Lookup rules work on overload sets, not on individual
> functions. If you define a ufcs method that overloads with a straight method, > you cannot call the ufcs method, this is intended.

I'm not sure that I agree that that's good behavior, but given how prevalent UFCS is and how many consider it good practice to use it rather than the normal function call syntax, it seems like it's a bad idea to name any member functions with a name which is known to conflict with a free function that is likely to be used with that type.

And given that unless we use UFCS as a matter of course, there really isn't a way for types to provide specialized implementations for well-known free functions (such as retro), I'm very much inclined to argue that we should just be using UFCS as much as possible and that users should be advised not to name their member functions with names which are likely to conflict with Phobos functions which are likely to be used with their types.

Though honestly, I'm inclined to argue that erroring out on foo.retro() because
typeof(foo) defines retro(int) is ultimately a bad idea. It risks making UFCS a
nightmare to use.

And I have a hard time believing that it's going to scale very well to have templated free functions check to see whether the type that they're being instantiated with declared a member function which matches. There's nothing special about retro. It's just one case among many where it would make sense for a type to provide a specialization for a well-known free function.

Certainly, it seems to me that this situation is exactly what UFCS was designed for. You call the function without caring whether it's a free function or a member function, and it just works. If the overloading rules cause problems with that, then we have a serious problem IMHO, and personally, I think that this case is the main reason why UFCS makes any objective sense at all - i.e. making it so that you can call a function without caring about whether it was defined as a free function or a member function. Every other reason to use UFCS is subjective.

--

https://issues.dlang.org/show_bug.cgi?id=12583

--- Comment #7 from Steven Schveighoffer <schveiguy@yahoo.com> ---
I've altered my opinion slightly on this. See: http://forum.dlang.org/post/op.xeuot6g2eav7ka@stevens-macbook-pro-2.local

Long story short, I think this is still a good idea, but the member function should NOT be named retro.

--

https://issues.dlang.org/show_bug.cgi?id=12583

Iain Buclaw <ibuclaw@gdcproject.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P1                          |P4

--