Now that's a DIP that could use some love

[Jonathan M Davis]

On Thursday, September 17, 2020 3:55:22 PM MDT Andrei Alexandrescu via 
Digitalmars-d wrote:
> On 9/17/20 5:39 PM, Jonathan M Davis wrote:
> > private traits in the public constraints. But even then, the actual
> > template constraints are pretty short, and the traits used are
> > descriptive enough that I wouldn't expect them to be hard to understand
> > or that repeating them in English text would help much.
>
> Here goes, straight from the horse's mouth
> (https://dlang.org/library/std/algorithm/iteration/each.each.html):
>
> Flag!"each" each(Range) (
>    Range r
> )
> if (!isForeachIterable!Range && (isRangeIterable!Range ||
> __traits(compiles, typeof(r.front).length)));
>
> Flag!"each" each(Iterable) (
>    auto ref Iterable r
> )
> if (isForeachIterable!Iterable || __traits(compiles,
> Parameters!(Parameters!(r.opApply))));
>
> Compare to "`each` requires type MyRange to work with foreach".
>
> Let's see:
>
> "pretty short" .................................................. FAIL
> "descriptive enough" ............................................ FAIL
> "wouldn't expect them to be hard to understand"  ................ FAIL
> "repeating them in English text wouldn't help much" ............. FAIL

The constraints themselves are short and look pretty clear to me:

> if (!isForeachIterable!Range && (isRangeIterable!Range ||
> __traits(compiles, typeof(r.front).length)));

> if (isForeachIterable!Iterable || __traits(compiles,
> Parameters!(Parameters!(r.opApply))));

Now, they could definitely use some improvement (well, a lot of
improvement), but I don't think that they're particularly obtuse even if
they're poorly written. On the other hand, I don't see how

"`each` requires type MyRange to work with foreach"

would be particularly useful information. What's MyRange? It sounds like a
roundabout way of saying that each requires a range, which isn't what it
actually requires at all (even it arguably should be what it requires). What
it really requires is a type that's iterable with foreach where the given
function can be called with the element type of the iterable.

The current template constraints test that the type is iterable with foreach
and have a branch for ranges and one for other types that work with foreach.
And having branches complicates things (also having isForeachIterable be
false for ranges makes it a poor name, though it's clear enough what's meant
from the context). In addition, they fail to test that the function is
callable with the element type, leaving that for the static ifs inside the
function overloads, which is definitely a bug, since then you can get a
compilation error within the function even if the arguments pass the
constraint. So, even if the constraints are understandable, they obviously
need work.

Given the traits that we currently have, the template constraint should
probably be something like

if (__traits(compiles, { foreach(e; r) { unaryFun!fun(e); } }) ||
    __traits(compiles, { foreach(i, e; r) { binaryFun!fun(i, e); } }))

and if we really need an overload because of the auto ref on the second
overload, then it should probably be something like

if (isInputRange!Range && __traits(compiles, unaryFun!fun(r.front)))

and

if (!isInputRange!Iterable &&
    (__traits(compiles, { foreach(e; r) { unaryFun!fun(e); } }) ||
     __traits(compiles, { foreach(i, e; r) { binaryFun!fun(i, e); } })))

In any case, regardless of what the exact template constraint is, let's say
that we had a message to go along with it that said what was required in
English. e.g.

"`each` requires a type which is iterable with foreach and whose element
 type is callable with the function provided as the template argument."

That doesn't really encompass the weirdness with the binary version, but it
gets across the unary version clearly enough and makes it clear what you
need to pass to each when the provided function is unary. However, IMHO, it
still isn't very useful, because if it fails, it doesn't tell me what I'm
doing wrong.

I should already know the abstract requirements of each if I'm calling it,
so the message is basically just telling me that my arguments are not
meeting the abstract requirements that I already knew about. At best, it's
helping me undersand what those abstract requirements are if I tried to use
each without actually reading the documention. But regardless, it doesn't
actually tell me what's wrong, just like seeing the template constraint
itself doesn't actually tell me what's wrong.

What I need to know is _why_ my arguments are not meeting the function
template's requirements. And unless that's really obvious by just glancing
at the code, because I made a simple and obvious mistake, that probably
means that I'm going to have to copy the template constraint into my own
code and test each piece individually to see which pieces are true and which
are false. And once I know that, I may have to dig deeper and go inside the
traits that were used in the template constraint and copy their contents
into my code so that I can check each piece individually to see what's true
and what's false so I can figured out where the problem is.

An English message may help you understand a bad template constraint, but if
a template constraint is so hard to understand that it needs a message in
English, then it generally means that either the function itself is overly
complicated, we're missing a trait for a general concept that we should have
a trait for, and/or the function template is overloaded when the differences
should be inside the template with static if branches instead of being in
the template constraints of overloads. In general, having

if (cond && stuff)

and

if (!cond && otherStuff)

is an anti-pattern and should be avoided where possible.

With a properly cleaned up template constraint, an English message would
just be rewording the constraint into English. So, it wouldn't be clarifying
much, and you would still need to look at the actual template constraint
(and possibly the implementations of the traits that it uses) in order to
know why the template constraint was failing. So, as far as I can tell,
unless a template constraint is badly written, the English message would be
redundant and unhelpful - and if it's shown instead of the actual template
constraint, then it's actively making it harder to get at the information
needed to figure out what's wrong.

If the compiler is going to make life easier when a template constraint
fails, then it needs to be giving information about what specifically in the
constraint is true and false so that you don't have to copy it into your
code and compile it to figure it out. Additionally, when it gags an error
within a template constraint, it would be useful to know what it gagged so
that you don't have to copy the pieces out to get that information. And all
of that relates to understanding the actual template constraint, what
exactly it's testing, and exactly which pieces are failing, not the abstract
concept that a message in English would provide. With a well-written
template constraint, the abstract concept and the template constraint itself
would be as close as reasonably possible, but regardless of whether an
additional message translating the constraint into English is provided, it's
still the actual template constraint that you need information about in
order to debug the problem.

- Jonathan M Davis