On Thursday, 17 September 2020 at 19:34:49 UTC, Andrei Alexandrescu wrote:
> On 9/17/20 3:16 PM, Jonathan M Davis wrote:
> This has got to be the ultimate in reverse psychology. Picture this: I set out to save Walter some work. I took what seemed a motherhood and apple pie argument - I mean, who in the world would argue against better error messages? Yes, in Queen's English! It turns out, those can be found.

That's a strawman. If all you actually were doing was making better error messages, then no one would argue against it. What you are proposing is adding a new feature, with new syntax that runs counter to existing facilities of the language. Introducing complexity and debt, while ultimately still relying on the user to know what they are doing to produce those better error messages. Which, if someone knows what they are doing is able to accomplish this now to the same degree as the proposed new feature.

On Thu, Sep 17, 2020 at 01:16:13PM -0600, Jonathan M Davis via Digitalmars-d wrote: [...]
> Honestly, IMHO, that makes the code far worse. You're just repeating the constraints in plain English, which requires more than double the code for each constraint and doesn't really make the error messages much easier to understand IMHO - especially when the constraints are already using traits that make what they're doing clear.
[...]
> It's like commenting every line to say what it does.

Exactly, this breaks DRY.


> And as Adam pointed out, the problem is usually in figuring out why a particular constraint is failing, not what a constraint means, and that has a tendency to result you having to do stuff like copy the constraint into your code and breaking it apart to test each piece and then potentially digging into the traits that it's using (such as isInputRange) to figure out why that particular trait is true or false.
[...]

That's totally been my experience with writing heavy range-based UFCS code.  These days, thanks to recent efforts to improve error messages, the compiler will now come back with "no matching overload: must satisfy constraints: X, Y, Z".  Which is a good start; however, it doesn't go far enough, because these days Phobos sig constraints are opaque:

	auto somePhobosFunc(R)(R r)
		if (isInputRange!R && satisfiesX!R && satisfiesY!R && ...)

The compiler tells me my argument fails the constraint `satisfiesX!R`, but `satisfiesX` is an internal Phobos helper template; I have no idea what it's checking for and why my argument fails to satisfy it.  At the end of the day, I have to go digging into Phobos code, copy-n-paste it into my own, then eliminate the failing conditions one by one, just like Jonathan says.

Instead of this half-baked hack of essentially writing comments that repeat what the code already says, what we should be doing is to take the current error messages one step further: when some clause in a sig constraint fails, how about the compiler ungags the errors that cropped up while evaluating that clause?  Instead of blindly ungagging *everything* (*cough*-verrors=spec*cough*), which results in a deluge of irrelevant errors that the one relevant message gets lost in, ungag only the most likely relevant errors: the ones produced when evaluating a failed sig constraint clause.

I think that would improve error messages much more than the repeat-after-yourself exercise proposed here.


T

-- 
Obviously, some things aren't very obvious.

On Thursday, September 17, 2020 1:34:49 PM MDT Andrei Alexandrescu via Digitalmars-d wrote:
> On 9/17/20 3:16 PM, Jonathan M Davis wrote:
> > Honestly, IMHO, that makes the code far worse. You're just repeating the constraints in plain English, which requires more than double the code for each constraint and doesn't really make the error messages much easier to understand IMHO - especially when the constraints are already using traits that make what they're doing clear.
>
> This has got to be the ultimate in reverse psychology. Picture this: I set out to save Walter some work. I took what seemed a motherhood and apple pie argument - I mean, who in the world would argue against better error messages? Yes, in Queen's English! It turns out, those can be found.
>
> Now imagine I wrote this instead:
>
> How about we do some DIP cleanup? All those dead and moribund DIPs laying around. I suggest we start with https://github.com/dlang/DIPs/pull/131 - it really does nothing else but repeat the constraints in natural language. There's also been improvements in how the compiler displays errors. I propose we just chop that DIP.

I really don't think that repeating a constraint in English improves the error messages particularly, and as shown in your example, it gets pretty verbose. In practice, I've never seen reading the constraint as the problem when dealing with a failed template constraint. The constraint itself already explains itself in code. The problem is figuring out which part is failing and why, and that usually requires digging deeper. IMHO, _that_ is the problem that would be useful to solve if we want to make it easier to solve problems with failed template constraints. Simply repeating the constraint in English doesn't do that. The only part of that DIP that looks useful to me is the fact that it breaks up the constraint so that you can see which part failed, but simply having the compiler break out the constraint along each && clause and print whether it was true or false would do the same thing without making a lot of template constraints more verbose.

I'd be fine with the DIP being chopped. IMHO, it doesn't realy help with the real problem.

- Jonathan M Davis

On 9/17/20 3:58 PM, H. S. Teoh wrote:
> On Thu, Sep 17, 2020 at 01:16:13PM -0600, Jonathan M Davis via Digitalmars-d wrote:
> [...]
>> Honestly, IMHO, that makes the code far worse. You're just repeating
>> the constraints in plain English, which requires more than double the
>> code for each constraint and doesn't really make the error messages
>> much easier to understand IMHO - especially when the constraints are
>> already using traits that make what they're doing clear.
> [...]
>> It's like commenting every line to say what it does.
> 
> Exactly, this breaks DRY.

Not at all! What in the world...? The constraint is the mechanics, it often says little about the high-level requirements. Granted, sometimes the mechanism is simple enough to be sufficiently evocative. But would you really like the compiler error messages in terms of the LR step that failed?

It's a funny coincidence this thread is going in parallel with a couple of other developments:

* `each` has literally inscrutable constraints (I say "literally" because their documentation is not visible)
* `equals` also has nigh unreadable constraints, see https://github.com/dlang/phobos/pull/7635/files

If you (cut and) DRY that stuff, I'll eat it.

I found these in literally the first two files I looked at in Phobos.

I can't believe I need to argue this stuff. DRY? No, it's wet like a drowned rat.

On Thursday, 17 September 2020 at 19:58:45 UTC, H. S. Teoh wrote:
> Instead of this half-baked hack of essentially writing comments that repeat what the code already says, what we should be doing is to take the current error messages one step further: when some clause in a sig constraint fails, how about the compiler ungags the errors that cropped up while evaluating that clause?
>  Instead of blindly ungagging *everything* (*cough*-verrors=spec*cough*), which results in a deluge of irrelevant errors that the one relevant message gets lost in, ungag only the most likely relevant errors: the ones produced when evaluating a failed sig constraint clause.

+1

The compiler is literally *already doing* all of the necessary work to determine why a given constraint failed. We just need it to *show* us that information in a way that's actually usable.

On Thursday, September 17, 2020 2:28:21 PM MDT Andrei Alexandrescu via Digitalmars-d wrote:
> On 9/17/20 3:58 PM, H. S. Teoh wrote:
> > On Thu, Sep 17, 2020 at 01:16:13PM -0600, Jonathan M Davis via Digitalmars-d wrote: [...]
> >
> >> Honestly, IMHO, that makes the code far worse. You're just repeating the constraints in plain English, which requires more than double the code for each constraint and doesn't really make the error messages much easier to understand IMHO - especially when the constraints are already using traits that make what they're doing clear.
> >
> > [...]
> >
> >> It's like commenting every line to say what it does.
> >
> > Exactly, this breaks DRY.
>
> Not at all! What in the world...? The constraint is the mechanics, it often says little about the high-level requirements. Granted, sometimes the mechanism is simple enough to be sufficiently evocative. But would you really like the compiler error messages in terms of the LR step that failed?
>
> It's a funny coincidence this thread is going in parallel with a couple of other developments:
>
> * `each` has literally inscrutable constraints (I say "literally"
> because their documentation is not visible)
> * `equals` also has nigh unreadable constraints, see
> https://github.com/dlang/phobos/pull/7635/files
>
> If you (cut and) DRY that stuff, I'll eat it.
>
> I found these in literally the first two files I looked at in Phobos.
>
> I can't believe I need to argue this stuff. DRY? No, it's wet like a drowned rat.

The problems with each are that it supports too many things and that it uses 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. Their names are basically just a truncated version of what I'd expect the English text to say anyway. Where things really get ugly is all of the static if conditions, but those are internal and shouldn't affect the user.

Either way, if each really just supported ranges and required that wrappers or explicit conversions be used for anything else, then the private traits wouldn't be required in the constraint, and the constraint would be straightforward.

- Jonathan M Davis

On Thu, Sep 17, 2020 at 04:28:21PM -0400, Andrei Alexandrescu via Digitalmars-d wrote:
> On 9/17/20 3:58 PM, H. S. Teoh wrote:
[...]
> > Exactly, this breaks DRY.
> 
> Not at all! What in the world...? The constraint is the mechanics, it often says little about the high-level requirements. Granted, sometimes the mechanism is simple enough to be sufficiently evocative. But would you really like the compiler error messages in terms of the LR step that failed?

A sig constraint that contains the equivalent of an LR step does not belong in the constraint, it belongs in a static if inside the function body.  Or at the very least, it belongs in a separately-documented template with a descriptive name that indicates what exactly it's testing for.

The sig constraint is meant for the outside world to read and digest. Why do you think we write:

	auto myRangeFunc(R)(R r)
		if (isInputRange!R)
	{ ... }

instead of:

	auto myRangeFunc(R)(R r)
		if (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)))
	{ ... }

?

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.


> It's a funny coincidence this thread is going in parallel with a couple of other developments:
> 
> * `each` has literally inscrutable constraints (I say "literally" because their documentation is not visible)

So the problem is (1) the constraints ought to be publicly documented, and (2) if that doesn't make sense, then .each ought to be rewritten with a different charter, as you said yourself.  How exactly does adding a string description to .each's sig constraints help in any way? Wouldn't they just repeat what the names of the clauses already say?

(And if the names are not descriptive enough, that's all the more reason to write a fuller description in the docs for the helper templates, which in either case should be made public.)


> * `equals` also has nigh unreadable constraints, see https://github.com/dlang/phobos/pull/7635/files
> 
> If you (cut and) DRY that stuff, I'll eat it.

This is a typical ailment that plagues Phobos code: the sig constraints expose implementation details that ought to be handled by static ifs inside a common function body, instead of a bunch of needless overloads with sig constraints that are completely irrelevant to the user.

As far as the user is concerned, all that matters to be able to use .equal is:

1) It takes two ranges.

2) The predicate is a binary function that compares the elements of the respective ranges.

These are the only two clauses that ought to appear in the sig constraints.

Everything else belongs in static ifs inside the function body (with a suitable else clause at the end that static asserts false, with a message explaining why the argument types could not be supported).  All that stuff about infinite ranges and forward ranges and whatever else is in there, is implementation details that are completely irrelevant to the user.  Why should the user care whether Phobos implements .equal as 5 different blocks of code, respectively taking input, forward, infinite, transfinite ranges, vs. one block of code that can handle any range?  That's the Phobos maintainers' problem, it's implementation details that users do not care, and do not want to care, about.  Such things do not belong in the sig constraints, but inside the function body together with the rest of the implementation details.

Doing it this way will:

1) Make the sig constraints self-evident so that you don't need to explain it in excruciating detail, esp. not write a string that essentially paraphrases what the sig constraint already says;

2) Eliminate useless overloads that ought not to be part of the user-facing API in the first place -- this will reduce the size of template error messages by reducing the number of overloads (and their associated text-dumps) printed by the compiler, and make the code easier to maintain (have you ever tried to debug std.conv.to? Good luck finding which of the 15 or so overloads is the one you're looking for).

3) Replace generic useless "could not match template" errors with a static assert message that explains exactly why the given arguments cannot work.  HERE is where your string message should be, not in the sig constraints.


So here it is.  All cut and dried for you. Now will you eat it? ;-)


T

-- 
You have to expect the unexpected. -- RL

On 9/17/20 5:40 PM, H. S. Teoh wrote:
> How exactly does adding
> a string description to .each's sig constraints help in any way?
> Wouldn't they just repeat what the names of the clauses already say?

Emphatically NO!

How is this:

mymodule.d(4): Error: template onlineapp.main.each!((x) => writeln(x)).each cannot deduce function from argument types !()(int), candidates are:
/dlang/dmd/linux/bin64/../../src/phobos/std/algorithm/iteration.d(962):       each(Range)(Range r)
  with Range = int
  must satisfy one of the following constraints:
       isRangeIterable!Range
       __traits(compiles, typeof(r.front).length)
/dlang/dmd/linux/bin64/../../src/phobos/std/algorithm/iteration.d(1023):        each(Iterable)(auto ref Iterable r)
  with Iterable = int
  must satisfy one of the following constraints:
       isForeachIterable!Iterable
       __traits(compiles, Parameters!(Parameters!(r.opApply)))

the same as this:

mymodule.d(42): "Attempting to use `each` with type `int`, which cannot be iterated using a foreach loop."

?

I mean how can anyone not chuckle at this crap? Did you guys gang on me with a prank?

https://run.dlang.io/is/cbO30I

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

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.