Generality creep

[Andrei Alexandrescu]

Walter and I were looking at indexOf in the standard library. It has 
several overloads, of which let's look at the first two:

https://dlang.org/library/std/string/index_of.html

Now, why couldn't they be merged into a single function with this 
constraint (their bodies are identical):

ptrdiff_t indexOf(Range) (
   Range s,
   dchar c,
   CaseSensitive cs = Yes.caseSensitive
)
if (isInputRange!Range && isSomeChar!(ElementType!Range));

It makes sense to look for a character in any range of characters.

Attempting to build that fails because of this type fails to work:

     struct TestAliasedString
     {
         string get() @safe @nogc pure nothrow { return _s; }
         alias get this;
         @disable this(this);
         string _s;
     }

The intuition is that the function should be general enough to figure 
out that, hey, TestAliasedString is kinda sorta a subtype of string. So 
the call should work.

So let's see why it doesn't work - i.e. why is TestAliasedString an 
input range? The definition of isInputRange (in std.range.primitives) is:

enum bool isInputRange(R) =
     is(typeof(R.init) == R)
     && is(ReturnType!((R r) => r.empty) == bool)
     && is(typeof((return ref R r) => r.front))
     && !is(ReturnType!((R r) => r.front) == void)
     && is(typeof((R r) => r.popFront));

Turns out the second clause fails. That takes us to the definition of 
empty in the same module:

@property bool empty(T)(auto ref scope const(T) a)
if (is(typeof(a.length) : size_t))
{
     return !a.length;
}

The intent is fairly clear - if a range defines empty as a size_t 
(somewhat oddly relaxed to "convertible to size_t"), then empty can be 
nicely defined in terms of length. Cool. But empty doesn't work with 
TestAliasedString due to an overlooked matter: the "const". A mutable 
TestAliasedString converts to a string, but a const or immutable 
TestAliasedString does NOT convert to a const string! So this fixes that 
matter:

     struct TestAliasedString
     {
         string get() @safe @nogc pure nothrow { return _s; }
         const(string) get() @safe @nogc pure nothrow const { return _s; }
         alias get this;
         @disable this(this);
         string _s;
     }

That makes empty() work, but also raises a nagging question: what was 
the relationship of TestAliasedString to string before this change? 
Surely that wasn't subtyping. (My response would be: "Odd.") And why was 
Phobos under the obligation to cater for such a type and its tenuous 
relationship to a range?

But wait, there's more. Things still don't work because of popFront. 
Looking at its definition:

void popFront(C)(scope ref inout(C)[] str) @trusted pure nothrow
if (isNarrowString!(C[]))
{ ... }

So, reasonably this function takes the range by reference so it can 
modify its internals. HOWEVER! The implementation of 
TestAliasedString.get() returns an rvalue, i.e. it's the equivalent of a 
conversion involving a temporary. Surely that's not to match, whether in 
the current language or the one after the rvalue DIP.

The change that does make the code work is:

     struct TestAliasedString
     {
         ref string get() @safe @nogc pure nothrow { return _s; }
         ref const(string) get() @safe @nogc pure nothrow const { return 
_s; }
         alias get this;
         @disable this(this);
         string _s;
     }

This indeed does implement a subtyping relationship, and passes the 
isInputRange test.

What's the moral of the story here? Generality is good, but it seems in 
several places in phobos (of which this is just one example), a 
combination of vague specification and aiming for a nice ideal of "work 
with anything remotely reasonable" has backfired into a morass of 
inconsistently defined and supported corner cases.

For this case in particular - I don't think we should support all types 
that support some half-hearted form of subtyping, at the cost of 
reducing generality and deprecating working code.