Generality creep

[Rubn]

On Tuesday, 19 March 2019 at 02:52:34 UTC, Andrei Alexandrescu 
wrote:
> 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.

So basically one more reason to avoid const entirely?