On Thursday, 17 September 2020 at 22:29:50 UTC, jmh530 wrote:
> On Thursday, 17 September 2020 at 21:55:22 UTC, Andrei Alexandrescu wrote:
>> [snip]
>
> I don't think the people who disagree and instead think that the situation is good. HS Teoh recommended an alternative approach that may provide simpler error reports. His approach would effectively widen the template constraints to something like just isInputRange. One could still make an argument about isInputRange not provided valuable error reporting, but that is one thing that Atila's concepts library attempts to fix.

I also want to draw some attention to another possible solution to this morass, which is enhancing template alias deduction. Someone was working on a DIP for this, but their approach became tricky in the more general case. Ideally something like below could work where the call to foo(y) would produce the result from the static assert. In other words, instantiating foo with T should cause ArrayType(T) to be instantiated, which could either produce the relevant alias that would be used for deduction or the result of the static assert appears before any additional matching would go on.

A similar template alias for InputRange (instead of isInputRange) would basically return the type if it passes the relevant checks and a detailed error message otherwise.

I would think this would somehow fit in with Stefan Koch's work on type/alias functions...

template ArrayType(T)
{
    import std.traits: isDynamicArray;
    static if (isDynamicArray!T) {
        alias ArrayType = T;
    } else {
        static assert(0, "T is not a valid ArrayType");
    }
}

void foo(T)(ArrayType!T x) {
 	
}

void main()
{
    int[] x;
    foo(x); //does not currently compile
    int y;
    foo(y); //does not currently compile, shouldn't compile
}

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

On Thu, Sep 17, 2020 at 08:43:02PM -0600, Jonathan M Davis via Digitalmars-d wrote: [...]
> 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.

Yes, and *this* is the crux of the issue here.  Get this right, and the rest is just cosmetic cleanup.  Get this wrong, and no matter how many cosmetics you pile on, it's still a carcass.

This is why I suggested that the compiler should ungag errors for the constraint clause that failed to be met.  *That* would be a lot more useful because it will actually tell me what went wrong, instead of papering over it with some generic abstract message that, as Jonathan says, I should have already known before calling the function.

Quite often, when there's a typo or some kind of bug in my code, the error comes in the form of some lambda or range failing to meet some sig constraints of a Phobos function.  The sig constraint may be checking that something is a forward range.  I *already* know that the function is expecting a forward range; and I wrote the UFCS chain believing that the resulting of the preceding part of the chain returns a forward range, and now it comes back and says "function XYZ expects a forward range".  Well no kidding, I already knew that.  What I need to know is, *why* is the preceding part of the UFCS chain *not* a forward range when I expected it to be one?  For this, I want to see what exactly about a forward range's requirements I failed to meet. Was it because I left out .save again?  Or was it because I had a typo and .front doesn't compile, so it failed to qualify as a forward range?

Currently, these failures are gagged, and the compiler substitutes in its place "argument must satisfy constraint: isForwardRange!R", which is of no help.  Even if this were replaced with a hand-written message (in Queen's English, no less), it would still be of no help.  Just stop dithering about, and TELL ME THE DARNED COMPILE ERROR DAMMIT!!!!!  I'm a programmer; I won't get scared by messages like "syntax error in myRange.front: no such identifier 'xyz'".  Or "cannot call .front from @safe code because it escapes a reference to member .zzz".  In fact, these are the very errors that will actually tell me what I need to fix in my code.  These are the errors that I *want* to see.

Substituting these "ugly" errors with some veneer of Queen's English is no different from "argument must satisfy constraint: isForwardRange!R", which essentially amounts to "hahaha your range fails to be a forward range, and ninner-ninners, I'm not gonna tell you why!".


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

Exactly.


T

-- 
Don't throw out the baby with the bathwater. Use your hands...

On Thursday, 17 September 2020 at 21:40:46 UTC, H. S. Teoh wrote:
> Precisely because the latter is the mechanics, the former is the high-level requirements.  If your sig constraints look like the latter such that you need a string description for it, then perhaps you should be thinking about refactoring it into a helper template with a descriptive name instead.

In general it's not practical to always (or even usually) write helper template traits, there can be a combinatorial explosion. Reading the name of such templates can be inferior to the logic of what they actually do. Helper templates should only be written if they'll be reused several times, and they need to have a clear, fairly simple concept.

On 17.09.20 23:55, Andrei Alexandrescu wrote:
> 
> Flag!"each" each(Range) (
>    Range r
> )
> if (!isForeachIterable!Range && (isRangeIterable!Range || __traits(compiles, typeof(r.front).length)));

So `each` says it works for types that cannot be iterated with `foreach` but whose `front` has a `length` member? :-)

Turns out it does not:

----
import std.algorithm;
struct S{
    struct T{
        struct Q{}
        Q length;
    }
    T front;
}

void main(){
    S().each!((x){});
}
----
std/algorithm/iteration.d(966): Error: template `std.range.primitives.empty` cannot deduce function from argument types `!()(S)`, candidates are:
...
----

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

On Thursday, 17 September 2020 at 23:10:23 UTC, jmh530 wrote:
>>
> template ArrayType(T)
> {
>     import std.traits: isDynamicArray;
>     static if (isDynamicArray!T) {
>         alias ArrayType = T;
>     } else {
>         static assert(0, "T is not a valid ArrayType");
>     }
> }
>
> void foo(T)(ArrayType!T x) {
>  	
> }
>
> void main()
> {
>     int[] x;
>     foo(x); //does not currently compile
>     int y;
>     foo(y); //does not currently compile, shouldn't compile
> }

Pretty cool that. It occurs to me that it may not be even necessary to declare the template parameter up front, it could just be...

void foo(ArrayType!T x)

That could be lowered / rewritten by the compiler back to..

void foo(T)(ArrayType!T x)

On Friday, 18 September 2020 at 09:32:29 UTC, claptrap wrote:
> [snip]
>
> Pretty cool that. It occurs to me that it may not be even necessary to declare the template parameter up front, it could just be...
>
> void foo(ArrayType!T x)
>
> That could be lowered / rewritten by the compiler back to..
>
> void foo(T)(ArrayType!T x)

I don't know. The compiler would need to know what T is. You might be special-casing a bit too much.

On Sunday, 13 September 2020 at 22:29:00 UTC, Andrei Alexandrescu wrote:
> https://github.com/dlang/DIPs/pull/131
>
> In essence:
>
> * Allow templates to accept multiple constraints.
> * Each constraint accepts the syntax "if (expr1, expr2)" where expr2 is a string describing the requirement.
> * The string corresponding to the constraint that fails is printed in the error message if no match is found.

If we're willing to rewrite most of the template constraints in Phobos, this might be workable:

enum string failConstraint(string name, string msg, Args...) = `alias %s = AliasSeq!(false, %s)`.format(Args[0], processArgs!(Args[1..$])); //processArgs is a helper template that converts types to string, etc...

template isInputRange(R)
{
    static if (!is(typeof(R.init) RInit == R))
        mixin(failConstraint!(isInputRange, `%s.init must have type %s, but it has type %s`, R, R, RInit);
    else static if (!is(ReturnType!((R r) => r.empty) REmpty == bool))
        mixin(failConstraint!(isInputRange, `ReturnType!((%s r) => r.empty) must be of type bool, but it is %s`, R, REmpty);
    else static if (...)
        ...etc.
    else
        enum isInputRange = true;
}

As long as constraints are able to be written in this form and the proposed `if (expr, msg)` constraint syntax allows tuples to be expanded over it, just like with functions.

On Friday, 18 September 2020 at 15:43:44 UTC, Meta wrote:
<snip>

That looks almost impossible to decipher through the newsgroup interface... I wish we had some sort of "formatted for code" equivalent like with Github et al.

On Friday, 18 September 2020 at 15:43:44 UTC, Meta wrote:
> enum string failConstraint(string name, string msg, Args...) = `alias %s = AliasSeq!(false, %s)`.format(Args[0], processArgs!(Args[1..$])); //processArgs is a helper template that converts types to string, etc...
>
> template isInputRange(R)
> {
>     static if (!is(typeof(R.init) RInit == R))
>         mixin(failConstraint!(isInputRange, `%s.init must have type %s, but it has type %s`, R, R, RInit);
>     else static if (!is(ReturnType!((R r) => r.empty) REmpty == bool))
>         mixin(failConstraint!(isInputRange, `ReturnType!((%s r) => r.empty) must be of type bool, but it is %s`, R, REmpty);
>     else static if (...)
>         ...etc.
>     else
>         enum isInputRange = true;
> }

Why not return a struct that has result of test and message for it (was mentioned before, in original discussion about multiple if constraints)?

It can also be improved to aggregate multiple error messages from different tests inside a template like isInputRange!T.

Regards,
Alexandru.