More complexity creep in Phobos

[Andrei Alexandrescu]

On 3/28/19 10:49 AM, Meta wrote:
> On Thursday, 28 March 2019 at 07:32:33 UTC, Andrei Alexandrescu wrote:
>> On 3/28/19 1:36 AM, Meta wrote:
>>> DirEntry states that it is a subtype of `string`
>>
>> No, please refer to the "Generality creep" thread.
> 
> Last time I checked, curt dismissal is not an argument.

Sorry. It was a pertinent response dressed as a curt dismissal. The 
opening post of the "Generality creep" is trivial to find and contains 
an explanation of exactly this matter. Walter posted a link to it, too 
(thanks). Here it is:

https://digitalmars.com/d/archives/digitalmars/D/Generality_creep_324867.html

What remains is that I copy and paste it for you, which I'll do at the 
end of this message.

I actually stopped reading your post following your assertion, because I 
inferred it would be increasingly wrong because it is based on a wrong 
assumption. Just now I went and read it, and indeed it is so.

> I maintain that 
> this is not "generality creep", but working around a broken language 
> feature.

You are wrong in a sense (factually that is not subtyping, it's 
coercion) and right in another (it's questionable to allow odd forms of 
alias this in the language in the first place). I wouldn't disagree 
either way. Depends on the angle.

Here's the text explaining why alias this to an rvalue is not subtyping:

> 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.