www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Better error messages - from reddit

reply Adam D. Ruppe <destructionator gmail.com> writes:
One of the reddit comments posted this link:

https://elm-lang.org/blog/compiler-errors-for-humans

Wow, those error messages are MUCH better than D has. Want my 
opinion on the best thing D should change for productivity? 
Completely overhaul the error messages.

The formatting in the link is nice, but what really wins is how 
it gives relevant details. "Missing age field" and "Html and 
String mismatch in array" sum up what it said - and I wish we had 
that in D too.

How many template constraint messages would be nice if it just 
said "missing popFront" instead of 40 lines of overload failed 
spam?

oh yeah that would be nice.
Mar 04 2019
next sibling parent Basile B. <b2.temp gmx.com> writes:
On Monday, 4 March 2019 at 14:50:11 UTC, Adam D. Ruppe wrote:

 One of the reddit comments posted this link:

 https://elm-lang.org/blog/compiler-errors-for-humans

 Wow, those error messages are MUCH better than D has. Want my 
 opinion on the best thing D should change for productivity? 
 Completely overhaul the error messages.

 The formatting in the link is nice, but what really wins is how 
 it gives relevant details. "Missing age field" and "Html and 
 String mismatch in array" sum up what it said - and I wish we 
 had that in D too.

 How many template constraint messages would be nice if it just 
 said "missing popFront" instead of 40 lines of overload failed 
 spam?

 oh yeah that would be nice.


Among one of them:

https://issues.dlang.org/buglist.cgi?component=dmd&keywords=diagnostic%2C%20&keywords_type=allwords&list_id=225094&product=D&query_format=advanced&resolution=---
Mar 04 2019
prev sibling next sibling parent reply "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Mon, Mar 04, 2019 at 02:50:11PM +0000, Adam D. Ruppe via Digitalmars-d wrote:
[...]

 How many template constraint messages would be nice if it just said
 "missing popFront" instead of 40 lines of overload failed spam?


[...]

This is why I'm starting to think sig constraints are not the genius
idea they first appeared to be. The basic problem is that when you write
a sig constraint, you're basically saying "I only accept template
arguments if they don't cause an error, otherwise it's not my problem
and somebody else can pick up the tab", whereas the essence of
user-friendly error-reporting is "I accept everything that looks like it
ought to work, and if something breaks, I'll tell you why it didn't
work".

With the former, you're essentially absolving yourself from the
responsibility of dealing with bad input by offloading it to other
overloads in the overload set. If nobody else in the overload set picks
up the tab, the compiler is left to clean up the mess, which it can't
possibly do a good job of because it wasn't hired to do this. I.e., it
doesn't understand the logical context of the error well enough to be
able to pinpoint and describe the problem in a user-friendly way.

To have user-friendly error reporting, the code that was intended to
pick up those bad arguments *need* to pick them up, and then report any
errors that were found -- which can then be made friendly because now
we're in the intended code context, and thereby have enough information
to emit a sensible message that makes sense in the intended context.

This is why years ago I said both here and in the Phobos PR queue that
sig constraints need to be written in such a way that anything that
*might* possibly be intended for that function ought to be accepted, and
static ifs should be used inside the function body to dispatch to the
code that handles the corresponding template arguments, with an
assert(0) at the end that tells you why the arguments weren't matched. A
human needs to write this, because auto-generated messages by the
compiler will never come up to the standard of user-friendliness.

This isn't the full solution, but it's at least a first stab at fixing
this problem. Ultimately, the philosophy behind sig constraints is
fundamentally incompatible with user-friendly error reporting, and
possibly a different approach needs to be adopted.


T

-- 
Study gravitation, it's a field with a lot of potential.
Mar 04 2019
next sibling parent reply Sebastiaan Koppe <mail skoppe.eu> writes:
On Monday, 4 March 2019 at 15:47:12 UTC, H. S. Teoh wrote:

 This is why I'm starting to think sig constraints are not the 
 genius idea they first appeared to be. The basic problem is 
 that when you write a sig constraint, you're basically saying 
 "I only accept template arguments if they don't cause an error, 
 otherwise it's not my problem and somebody else can pick up the 
 tab", whereas the essence of user-friendly error-reporting is 
 "I accept everything that looks like it ought to work, and if 
 something breaks, I'll tell you why it didn't work".


It's even worse. There might be another sig constraint that is 
perfectly happy. It might even be in another library. Your 
function has no way to know that. Ergo, error messages are hard.

With Rust a type declares to conform to a trait explicitly. Error 
messages are easy.

With D it seems we are reinventing traits, poorly. And all 
because we wanted to avoid naming them?
Mar 04 2019
next sibling parent reply "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Mon, Mar 04, 2019 at 04:49:25PM +0000, Sebastiaan Koppe via Digitalmars-d
wrote:

 On Monday, 4 March 2019 at 15:47:12 UTC, H. S. Teoh wrote:

 This is why I'm starting to think sig constraints are not the genius
 idea they first appeared to be. The basic problem is that when you
 write a sig constraint, you're basically saying "I only accept
 template arguments if they don't cause an error, otherwise it's not
 my problem and somebody else can pick up the tab", whereas the
 essence of user-friendly error-reporting is "I accept everything
 that looks like it ought to work, and if something breaks, I'll tell
 you why it didn't work".


 
 It's even worse. There might be another sig constraint that is
 perfectly happy. It might even be in another library. Your function
 has no way to know that. Ergo, error messages are hard.


Yep.  This can be good or bad: allowing this flexibility *can* mean that
the other library can extend your function without you needing to be
involved.

Of course, this flexibility comes at a pretty steep price.



 With Rust a type declares to conform to a trait explicitly. Error
 messages are easy.
 
 With D it seems we are reinventing traits, poorly. And all because we
 wanted to avoid naming them?


No, I think the original reasoning was that allowing arbitrary boolean
clauses in sig constraints is more expressive than allowing only a
subset of expressions (e.g., implementsTrait!T).  It's necessarily not
wrong per se, but the extra expressive power does come at a price. And
we've been paying for it over and over.

It's just like the tradeoff between stack machines and Turing machines.
Turing machines are powerful enough to express anything you might want
to compute, but it comes at the price that certain seemingly-simple
things become undecidable. If you're willing to give up a certain degree
of expressiveness, you can eliminate (some of) the undecidable problems.


T

-- 
Today's society is one of specialization: as you grow, you learn more and more
about less and less. Eventually, you know everything about nothing.
Mar 04 2019
parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 3/4/2019 9:19 AM, H. S. Teoh wrote:

 No, I think the original reasoning was that allowing arbitrary boolean
 clauses in sig constraints is more expressive than allowing only a
 subset of expressions (e.g., implementsTrait!T).


That's correct. For example, you could have a constraint that required an odd 
integer.

Use of `static if` and `pragma(msg)` can be used to generate custom error
messages:

     static if (!isInputRange(r)) pragma(msg, r, " must be an Input Range");

This is ultimately far more powerful than anything the compiler can come up 
with, because it can be about the user's context, not the compiler's.
Mar 04 2019
parent reply Nicholas Wilson <iamthewilsonator hotmail.com> writes:
On Tuesday, 5 March 2019 at 01:07:08 UTC, Walter Bright wrote:

 Use of `static if` and `pragma(msg)` can be used to generate 
 custom error messages:

     static if (!isInputRange(r)) pragma(msg, r, " must be an 
 Input Range");


That doesn't work if you need to overload against it because then 
you will cause a matches multiple template error.


 This is ultimately far more powerful than anything the compiler 
 can come up with, because it can be about the user's context, 
 not the compiler's.


That power and flexibility does however severely limit the 
compiler's ability to provide sensible error messages, 99.9% of 
constraints are in CNF because thats the only  reasonable way to 
compose and perform overload specialisation.

The DIP covers all the cases.
Mar 04 2019
parent Walter Bright <newshound2 digitalmars.com> writes:
On 3/4/2019 5:23 PM, Nicholas Wilson wrote:

 On Tuesday, 5 March 2019 at 01:07:08 UTC, Walter Bright wrote:

 Use of `static if` and `pragma(msg)` can be used to generate custom error 
 messages:

     static if (!isInputRange(r)) pragma(msg, r, " must be an Input Range");


 
 That doesn't work if you need to overload against it because then you will
cause 
 a matches multiple template error.


Frankly I think the use of overloads in D has gone too far. Anyhow, the above 
shows the principle. With some careful coding, it can work for the various 
scenarios.
Mar 04 2019
prev sibling next sibling parent rikki cattermole <rikki cattermole.co.nz> writes:
On 05/03/2019 5:49 AM, Sebastiaan Koppe wrote:

 With Rust a type declares to conform to a trait explicitly. Error 
 messages are easy.
 
 With D it seems we are reinventing traits, poorly. And all because we 
 wanted to avoid naming them?


Rust traits and Swift's Protocols are what I think of as inverse signatures.

The concept itself dates back to ML, where the signature has the 
capacity to adapt to the implementation to some degree. We can do this 
in D probably better than any other language and not require a horrible 
linking "type".

But the DIP is going to be massive and it will mean Phobos will get an 
almost complete redesign. Its not something I'm bringing to the table 
until I'm very sure that I have it completely nailed down and not just 
the basic semantics. It has potential for being D's killer feature. But 
it could also never make it out of community review. Its worth waiting 
for, now that these issues have matured quite a bit.
Mar 04 2019
prev sibling next sibling parent Meta <jared771 gmail.com> writes:
On Monday, 4 March 2019 at 16:49:25 UTC, Sebastiaan Koppe wrote:

 On Monday, 4 March 2019 at 15:47:12 UTC, H. S. Teoh wrote:

 This is why I'm starting to think sig constraints are not the 
 genius idea they first appeared to be. The basic problem is 
 that when you write a sig constraint, you're basically saying 
 "I only accept template arguments if they don't cause an 
 error, otherwise it's not my problem and somebody else can 
 pick up the tab", whereas the essence of user-friendly 
 error-reporting is "I accept everything that looks like it 
 ought to work, and if something breaks, I'll tell you why it 
 didn't work".


 It's even worse. There might be another sig constraint that is 
 perfectly happy. It might even be in another library. Your 
 function has no way to know that. Ergo, error messages are hard.

 With Rust a type declares to conform to a trait explicitly. 
 Error messages are easy.

 With D it seems we are reinventing traits, poorly. And all 
 because we wanted to avoid naming them?


I think that's a little drastic. Template constraints are far 
more powerful than Rust traits or Swift protocols.

I agree, though, that their design makes it much harder to give 
good error messages.
Mar 04 2019
prev sibling parent Atila Neves <atila.neves gmail.com> writes:
On Monday, 4 March 2019 at 16:49:25 UTC, Sebastiaan Koppe wrote:

 On Monday, 4 March 2019 at 15:47:12 UTC, H. S. Teoh wrote:

 [...]


 It's even worse. There might be another sig constraint that is 
 perfectly happy. It might even be in another library. Your 
 function has no way to know that. Ergo, error messages are hard.

 With Rust a type declares to conform to a trait explicitly. 
 Error messages are easy.

 With D it seems we are reinventing traits, poorly. And all 
 because we wanted to avoid naming them?


https://github.com/atilaneves/concepts

Not the same, but works well enough for me.
Mar 05 2019
prev sibling next sibling parent reply jmh530 <john.michael.hall gmail.com> writes:
On Monday, 4 March 2019 at 15:47:12 UTC, H. S. Teoh wrote:

 [snip]

 This is why years ago I said both here and in the Phobos PR 
 queue that sig constraints need to be written in such a way 
 that anything that *might* possibly be intended for that 
 function ought to be accepted, and static ifs should be used 
 inside the function body to dispatch to the code that handles 
 the corresponding template arguments, with an > assert(0) at 
 the end that tells you why the arguments weren't matched. [snip]


What would be an example of a signature constraint that you think 
would be worth keeping? I.e., why not do everything with static 
ifs?
Mar 04 2019
next sibling parent reply "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Mon, Mar 04, 2019 at 06:18:14PM +0000, jmh530 via Digitalmars-d wrote:

 On Monday, 4 March 2019 at 15:47:12 UTC, H. S. Teoh wrote:

 [snip]
 
 This is why years ago I said both here and in the Phobos PR queue
 that sig constraints need to be written in such a way that anything
 that *might* possibly be intended for that function ought to be
 accepted, and static ifs should be used inside the function body to
 dispatch to the code that handles the corresponding template
 arguments, with an > assert(0) at the end that tells you why the
 arguments weren't matched.  [snip]


 
 What would be an example of a signature constraint that you think
 would be worth keeping? I.e., why not do everything with static ifs?


Haha, good question.  My current stance (which may change -- I'm not
fully convinced either way yet) is a matter of documentation.  Static
ifs inside the function body are invisible to the user, since they are
implementation details, so they don't tell the user what is expected by
the function.  A sig constraint is part of the function signature, and
therefore a good place to document intent.

Note that intent may not be the same thing as implementation.  For
example, if myFunc logically expects an input range, but the current
implementation can only handle bidirectional ranges, then I'd say put
isInputRange!R in the sig constraints, and static assert(0) inside the
function body if R is not also a bidirectional range.  IOW, the intent
is that myFunc should accept all input ranges, but at the moment our
algorithm can only handle a subset of that.  Instead of cluttering the
sig constraint with unreadable clauses like "if this is an input range,
and it also has .length, and it also has feature X and trait Y, and it
also howls at the moon at night", just keep it simple and to-the-point:
"myFunc expects an input range". The rest of the dirty implementation
details can be documented in the ddoc comment and checked with static
ifs / static asserts inside.

Similarly, if two different algorithms are used depending on whether R
is an input range or a forward range, that should be done via static if
-- the user doesn't care whether your algorithm treats forward ranges
differently from input ranges; your function should present a unified,
logically-whole API rather than two similar-looking overloads with
inscrutable sig constraints.  And if the function logically ought to
also accept bidirectional ranges but your current algorithm for whatever
reason breaks in that case, the sig constraint should still declare that
it accepts all input ranges, but you'd have a static assert in the
function body that explains to the user exactly why bidi ranges aren't
currently supported.

Of course, this doesn't quite solve the problem of bad error messages
when you passed something that you thought was an input range, but fails
the isInputRange constraint.  But with simplified sig constraints of
this sort, it's easier to tell what went wrong:

	couldn't match call to myFunc, candidates are:
	auto myFunc(R) if (isInputRange!R)

is a lot easier to read, and figure out what went wrong, than:

	couldn't match call to myFunc, candidates are:
	auto myFunc(R) if (isInputRange!R && !isForwardRange)
	auto myFunc(R) if (isForwardRange!R && hasLength!R && howlsAtMoon!R)
	auto myFunc(R) if (!howlsAtMoon!R && (isInputRange!R || is(ElementType!R :
dchar)) && !isSomeString!R)

where you can't even tell at a glance which overload was the intended
one.

Once we've eliminated needless overloads from the equation, the problem
of emitting better error messages is simplified to emitting a sane error
message for one failed constraint of one overload, rather than N failed
constraints for M overloads.  For that, perhaps Andrei's idea of adding
string clauses to sig constraints might be one way of tackling this.

But in any case, there needs to be a way for a template like
isInputRange to emit a specific error like "R does not implement
.empty", rather than just silently failing (because errors are gagged --
D's version of SFINAE) and offloading to whatever other overloads there
may be.  Then when the compiler lists the candidate functions that
failed to match, it could also display this error message beside each
one to indicate what went wrong, where.


T

-- 
"I suspect the best way to deal with procrastination is to put off the
procrastination itself until later. I've been meaning to try this, but haven't
gotten around to it yet. " -- swr
Mar 04 2019
parent jmh530 <john.michael.hall gmail.com> writes:
On Monday, 4 March 2019 at 19:03:39 UTC, H. S. Teoh wrote:

 

  [snip]


Very clear. Good points.
Mar 04 2019
prev sibling parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 3/4/19 1:18 PM, jmh530 wrote:

 On Monday, 4 March 2019 at 15:47:12 UTC, H. S. Teoh wrote:

 [snip]

 This is why years ago I said both here and in the Phobos PR queue that 
 sig constraints need to be written in such a way that anything that 
 *might* possibly be intended for that function ought to be accepted, 
 and static ifs should be used inside the function body to dispatch to 
 the code that handles the corresponding template arguments, with an > 
 assert(0) at the end that tells you why the arguments weren't matched. 
 [snip]


 
 What would be an example of a signature constraint that you think would 
 be worth keeping? I.e., why not do everything with static ifs?


Overloads with the same arity within the same module should use an 
umbrella constraint (disjunction of all supported cases) and static if 
inside. It would be nice if someone made a pass through Phobos doing 
that throughout.

Again: SAME arity, SAME module, use the umbrella constraint that is a 
DISJUNCTION of supported cases, distinguish INSIDE with static if.
Mar 05 2019
prev sibling next sibling parent reply Adam D. Ruppe <destructionator gmail.com> writes:
On Monday, 4 March 2019 at 15:47:12 UTC, H. S. Teoh wrote:

 This is why I'm starting to think sig constraints are not the 
 genius idea they first appeared to be.


If the implementation actually told you which individual pieces 
failed (and perhaps even why), I don't think we'd be worried 
about this.

I have before desired color to be used there: a passing 
constraint gets highlighted one way, a failing constraint 
another, and short-circuited constraints get nothing special.

Even without formatting the output, this would very quickly give 
you a menu of what is and isn't working. And with formatting, it 
just gets better. (Though I also say I want XML error messages 
and a dedicated error viewer, so more and less detail is 
available upon request. But even plain text can give a LOT more 
actionable info than it does right now.)

Imagine what we'd say if the error message was not:

ooooo.d(4): Error: template std.algorithm.sorting.sort cannot 
deduce function from argument types !()(FilterResult!(unaryFun, 
int[])), candidates are:
/home/me/d/dmd2/linux/bin32/../../src/phobos/std/algorithm/sorting.d(1847):
    std.algorithm.sorting.sort(alias less = "a < b", SwapStrategy 
ss = SwapStrategy.unstable, Range)(Range r) if ((ss == 
SwapStrategy.unstable && (hasSwappableElements!Range || 
hasAssignableElements!Range) || ss != SwapStrategy.unstable &&
asAssignableElements!Range) && isRandomAccessRange!Range && 
hasSlicing!Range && hasLength!Range)

but rather



ooooo.d(4): Error: template std.algorithm.sorting.sort cannot 
deduce function from argument types
    !()(FilterResult!(unaryFun, int[])), candidates are:
/home/me/d/dmd2/linux/bin32/../../src/phobos/std/algorithm/sorting.d(1847):
    std.algorithm.sorting.sort([collapsed])(Range r)
       if (
         // passed
         (ss == SwapStrategy.unstable &&
         // passed
         (hasSwappableElements!Range || 
hasAssignableElements!Range) ||
          // short-circuited
         ss != SwapStrategy.unstable &&
asAssignableElements!Range)
         // ******* FAILED ***********
         && isRandomAccessRange!Range
         // short-circuited, but would fail
         && hasSlicing!Range && hasLength!Range
       )


That tells you everything you need to know.
Mar 04 2019
next sibling parent reply Nicholas Wilson <iamthewilsonator hotmail.com> writes:
On Monday, 4 March 2019 at 23:03:28 UTC, Adam D. Ruppe wrote:

 On Monday, 4 March 2019 at 15:47:12 UTC, H. S. Teoh wrote:

 This is why I'm starting to think sig constraints are not the 
 genius idea they first appeared to be.


 If the implementation actually told you which individual pieces 
 failed (and perhaps even why), I don't think we'd be worried 
 about this.


Way ahead of you ;)

https://github.com/dlang/DIPs/pull/131
Mar 04 2019
parent reply Adam D. Ruppe <destructionator gmail.com> writes:
On Monday, 4 March 2019 at 23:46:55 UTC, Nicholas Wilson wrote:

 https://github.com/dlang/DIPs/pull/131


Seems overkill to me, though it might be useful in some cases, 
the compiler already has enough information in current code. It 
just isn't telling us what it knows.

(that said i prolly wouldn't vote against it, i also don't see a 
need to change code to improve error messages.)
Mar 04 2019
parent reply "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Mon, Mar 04, 2019 at 11:54:34PM +0000, Adam D. Ruppe via Digitalmars-d wrote:

 On Monday, 4 March 2019 at 23:46:55 UTC, Nicholas Wilson wrote:

 https://github.com/dlang/DIPs/pull/131


 
 Seems overkill to me, though it might be useful in some cases, the
 compiler already has enough information in current code. It just isn't
 telling us what it knows.
 
 (that said i prolly wouldn't vote against it, i also don't see a need
 to change code to improve error messages.)


Yeah, I think the DIP would be a welcome improvement, but OTOH Adam's
idea already works without needing to change existing code, because the
compiler already knows what it knows.

Given that programmers tend to be lazy (why spend the time writing
elaborate sig constraints when I could be working on the function body
where the real work is done), having the compiler able to emit useful
information *without help from the programmer* is a big plus.

An even bigger plus is that it can be done today with just a little
change in the compiler.


T

-- 
Amateurs built the Ark; professionals built the Titanic.
Mar 04 2019
parent reply Nicholas Wilson <iamthewilsonator hotmail.com> writes:
On Tuesday, 5 March 2019 at 00:03:39 UTC, H. S. Teoh wrote:

 On Mon, Mar 04, 2019 at 11:54:34PM +0000, Adam D. Ruppe via 
 Digitalmars-d wrote:

 On Monday, 4 March 2019 at 23:46:55 UTC, Nicholas Wilson wrote:

 https://github.com/dlang/DIPs/pull/131


 
 Seems overkill to me, though it might be useful in some cases, 
 the compiler already has enough information in current code. 
 It just isn't telling us what it knows.
 
 (that said i prolly wouldn't vote against it, i also don't see 
 a need to change code to improve error messages.)


 Yeah, I think the DIP would be a welcome improvement, but OTOH 
 Adam's idea already works without needing to change existing 
 code, because the compiler already knows what it knows.


You still need to change the compiler, which is very hard because 
...


 Given that programmers tend to be lazy (why spend the time 
 writing elaborate sig constraints when I could be working on 
 the function body where the real work is done), having the 
 compiler able to emit useful information *without help from the 
 programmer* is a big plus.

 An even bigger plus is that it can be done today with just a 
 little change in the compiler.


... the problem (from the perspective of issuing a nice error) is 
that you can arbitrarily compose logic which makes sorting the 
what from the chaff extremely difficult and that signal to noise 
is very important (ever used -verrors=spec ?). Believe me I 
tried, saw a suggestion by aliak in a Phobos PR thread and 
thought that would make things so much easier, and thus arose 
that DIP.

The DIP also allows you to give a message if a particular 
subcontract failed. IMO they are much easier to look at (no need 
to match parens and keep track of && and ||'s .
Mar 04 2019
parent reply "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Tue, Mar 05, 2019 at 12:44:25AM +0000, Nicholas Wilson via Digitalmars-d
wrote:

 On Tuesday, 5 March 2019 at 00:03:39 UTC, H. S. Teoh wrote:


[...]

 Given that programmers tend to be lazy (why spend the time writing
 elaborate sig constraints when I could be working on the function
 body where the real work is done), having the compiler able to emit
 useful information *without help from the programmer* is a big plus.
 
 An even bigger plus is that it can be done today with just a little
 change in the compiler.


 
 ... the problem (from the perspective of issuing a nice error) is that
 you can arbitrarily compose logic which makes sorting the what from
 the chaff extremely difficult and that signal to noise is very
 important (ever used -verrors=spec ?). Believe me I tried, saw a
 suggestion by aliak in a Phobos PR thread and thought that would make
 things so much easier, and thus arose that DIP.


Well, if you're looking for a perfect solution, then yes this will be
very complicated and hairy to implement.

But to take care of the most common case, all we have to do is to assume
that sig constraints are of the form (A && B && C && ...).  The compiler
only needs to report which of these top level conjuncts failed.  If a
sig constraint isn't of this form, then fallback to reporting the entire
constraint as failed, i.e., treat it as the case (A) (single-argument
conjunction).

It's not a perfect solution, but having this is already a lot better
than the current pessimal state of things.  Your DIP represents an
improvement over this most basic step, but IMO we should at least have
this basic step first.  Don't let the perfect become the enemy of the
good yet again.


T

-- 
Amateurs built the Ark; professionals built the Titanic.
Mar 04 2019
next sibling parent reply Nicholas Wilson <iamthewilsonator hotmail.com> writes:
On Tuesday, 5 March 2019 at 01:18:05 UTC, H. S. Teoh wrote:

 Well, if you're looking for a perfect solution, then yes this 
 will be very complicated and hairy to implement.


Unfortunately both options are going to be nasty to implement 
because of the way that the compiler works, namely that you need 
to reevaluate the expressions and print their source. This sounds 
trivial, but because the constraint is boolean the result is 
overridden, so you need to look at the template declaration's 
constraint and reevaluate that, but then all the templates that 
are used in the expression are already instantiated so all you 
get from `isInputRange!R && isFoo!Bar` is `true && false` so you 
have to flush the template cache (which fortunately is fine 
because at this point you know the compiler is going to give an 
error). There is possibly a way to get back at the source from 
the location but I haven't got that far.


 But to take care of the most common case, all we have to do is 
 to assume that sig constraints are of the form (A && B && C && 
 ...).  The compiler only needs to report which of these top 
 level conjuncts failed.  If a sig constraint isn't of this 
 form, then fallback to reporting the entire constraint as 
 failed, i.e., treat it as the case (A) (single-argument 
 conjunction).


You don't actually get (A && B && C && ...) you get (A && (B && 
(C && ...))) (at least I think thats what you get I haven't been 
able to confirm the exact AST)
each of which could contain arbitrary conjugation, so you need to 
recurse and that hurts my head.


 It's not a perfect solution, but having this is already a lot 
 better than the current pessimal state of things.  Your DIP 
 represents an improvement over this most basic step, but IMO we 
 should at least have this basic step first.  Don't let the 
 perfect become the enemy of the good yet again.


Fear not, I'm going to use Dconf to get through all the 
bureaucracy (and get help with the implementation).
Mar 04 2019
next sibling parent reply Adam D. Ruppe <destructionator gmail.com> writes:
On Tuesday, 5 March 2019 at 01:49:26 UTC, Nicholas Wilson wrote:

 Unfortunately both options are going to be nasty to implement 
 because of the way that the compiler works, namely that you 
 need to reevaluate the expressions and print their source.



Oh, I'm not so sure it is that hard. Well, it took me 90 mins 
instead of the 30 I promised my wife, but the following patch 
kinda sorta does it, a proof of concept at least. Not well, this 
is by no means done, but it makes real progress on the status quo 
even in its incomplete state.

Given the test program:
---
import std.algorithm;
void main() {
	sort([1,2,3].filter!("a == 1"));
}
---

I get this output:

* * * * *

<snip some irrelevant spam from the top>

E1: E1: E1: E1: E1: cast(SwapStrategy)0 == cast(SwapStrategy)0
E2: E1: hasSwappableElements!(FilterResult!(unaryFun, int[]))



E2: isRandomAccessRange!(FilterResult!(unaryFun, int[]))/* was 
false */




/home/me/test/ooooo.d(16): Error: template 
std.algorithm.sorting.sort cannot deduce function from argument 
types !()(FilterResult!(unaryFun, int[])), candidates are:
../generated/linux/release/64/../../../../../phobos/std/algori
hm/sorting.d(1855):        std.algorithm.sorting.sort(alias
ess = "a < b", SwapStrategy ss = SwapStrategy.unstable, 
Range)(Range r) if ((ss == SwapStrategy.unstable && 
(hasSwappableElements!Range || hasAssignableElements!Range) || ss 
!= SwapStrategy.unstable && hasAssignableElements!Range) && 
isRandomAccessRange!Range && hasSlicing!Range && hasLength!Range)


* * * * *


It told me what was going on! The last line of the spam is quite 
valuable: it told me the isRandomAccessRange constraint failed... 
and it even told me it was passed a FilterResult!(), which helps 
more than you'd think when dealing with all kinds of hidden auto 
returns and stuff.


That's some real world code right here - people ask about 
basically that error every other week on this forum - and this 
error is a lot more legible.


I'm telling you, this is doable.


Patch against dmd master follows:




diff --git a/src/dmd/cond.d b/src/dmd/cond.d
index 793cefc..ece927b 100644
--- a/src/dmd/cond.d
+++ b/src/dmd/cond.d
   -855,7 +855,7    extern (C++) final class StaticIfCondition : 
Condition

              import dmd.staticcond;
              bool errors;
-            bool result = evalStaticCondition(sc, exp, exp, 
errors);
+            bool result = evalStaticCondition(sc, exp, exp, 
errors).result;

              // Prevent repeated condition evaluation.
              // See: fail_compilation/fail7815.d
diff --git a/src/dmd/dtemplate.d b/src/dmd/dtemplate.d
index 933df8d..b082dc3 100644
--- a/src/dmd/dtemplate.d
+++ b/src/dmd/dtemplate.d
   -684,10 +684,11    extern (C++) final class 
TemplateDeclaration : ScopeDsymbol
          return protection;
      }

+    import dmd.staticcond;
      /****************************
       * Check to see if constraint is satisfied.
       */
-    extern (D) bool evaluateConstraint(TemplateInstance ti, 
Scope* sc, Scope* paramscope, Objects* dedargs, FuncDeclaration 
fd)
+    extern (D) StaticConditionResult 
evaluateConstraint(TemplateInstance ti, Scope* sc, Scope* 
paramscope, Objects* dedargs, FuncDeclaration fd)
      {
          /* Detect recursive attempts to instantiate this 
template declaration,
           * https://issues.dlang.org/show_bug.cgi?id=4072
   -713,7 +714,7    extern (C++) final class TemplateDeclaration 
: ScopeDsymbol
                  for (Scope* scx = sc; scx; scx = scx.enclosing)
                  {
                      if (scx == p.sc)
-                        return false;
+                        return new StaticConditionResult(null, 
false, false);
                  }
              }
              /* BUG: should also check for ref param differences
   -788,13 +789,13    extern (C++) final class 
TemplateDeclaration : ScopeDsymbol
          ti.inst = ti; // temporary instantiation to enable 
genIdent()
          scx.flags |= SCOPE.constraint;
          bool errors;
-        bool result = evalStaticCondition(scx, constraint, e, 
errors);
+        auto result = evalStaticCondition(scx, constraint, e, 
errors);
          ti.inst = null;
          ti.symtab = null;
          scx = scx.pop();
          previous = pr.prev; // unlink from threaded list
          if (errors)
-            return false;
+            return new StaticConditionResult(null, false, false);
          return result;
      }

   -970,7 +971,7    extern (C++) final class TemplateDeclaration 
: ScopeDsymbol
              }

              // TODO: dedtypes => ti.tiargs ?
-            if (!evaluateConstraint(ti, sc, paramscope, 
dedtypes, fd))
+            if (!evaluateConstraint(ti, sc, paramscope, 
dedtypes, fd).result)
                  goto Lnomatch;
          }

   -2003,8 +2004,11    extern (C++) final class 
TemplateDeclaration : ScopeDsymbol

          if (constraint)
          {
-            if (!evaluateConstraint(ti, sc, paramscope, dedargs, 
fd))
+	    auto result = evaluateConstraint(ti, sc, paramscope, 
dedargs, fd);
+            if (!result.result) {
+	        printf("%s\n", result.toChars());
                  goto Lnomatch;
+            }
          }

          version (none)
diff --git a/src/dmd/semantic2.d b/src/dmd/semantic2.d
index 36aced3..92bab02 100644
--- a/src/dmd/semantic2.d
+++ b/src/dmd/semantic2.d
   -101,7 +101,7    private extern(C++) final class 
Semantic2Visitor : Visitor

          import dmd.staticcond;
          bool errors;
-        bool result = evalStaticCondition(sc, sa.exp, sa.exp, 
errors);
+        bool result = evalStaticCondition(sc, sa.exp, sa.exp, 
errors).result;
          sc = sc.pop();
          if (errors)
          {
diff --git a/src/dmd/staticcond.d b/src/dmd/staticcond.d
index 5521a37..acc6298 100644
--- a/src/dmd/staticcond.d
+++ b/src/dmd/staticcond.d
   -27,6 +27,48    import dmd.tokens;
  import dmd.utils;


+class StaticConditionResult {
+	this(Expression exp, bool result, bool wasEvaluated) {
+		this.exp = exp;
+		this.result = result;
+		this.wasEvaluated = wasEvaluated;
+	}
+
+	Expression exp; /// original expression, for error messages
+	bool result; /// final result
+	bool wasEvaluated; /// if this part was evaluated at all
+
+	StaticConditionResult e1; /// result on the one side, if done
+	StaticConditionResult e2; /// result on the other side, if done
+
+	const(char)* toChars() {
+		string s = this.toString();
+		s ~= "\n";
+		return s.ptr;
+	}
+
+	override string toString() {
+		import core.stdc.string;
+		string s;
+		if(e1) {
+			s ~= "E1: " ~ e1.toString() ~ "\n";
+		}
+
+		if(e2) {
+			s ~= "E2: " ~ e2.toString() ~ "\n";
+		}
+
+		if(!e1 && !e2) {
+			auto c = exp.toChars();
+
+			s ~= c[0 .. strlen(c)];
+			if(wasEvaluated && !result)
+				s ~= "/* was false */";
+		}
+
+		return s;
+	}
+}

  /********************************************
   * Semantically analyze and then evaluate a static condition at 
compile time.
   -42,37 +84,48    import dmd.utils;
   *      true if evaluates to true
   */

-bool evalStaticCondition(Scope* sc, Expression exp, Expression 
e, ref bool errors)
+StaticConditionResult evalStaticCondition(Scope* sc, Expression 
exp, Expression e, ref bool errors)
  {
+//if(e.parens) printf("PARANS %s\n\n", e.toChars);
      if (e.op == TOK.andAnd || e.op == TOK.orOr)
      {
          LogicalExp aae = cast(LogicalExp)e;
-        bool result = evalStaticCondition(sc, exp, aae.e1, 
errors);
+	auto result = new StaticConditionResult(e, false, true);
+        auto r2 = evalStaticCondition(sc, exp, aae.e1, errors);
+	result.e1 = r2;
+	result.result = r2.result;
          if (errors)
-            return false;
+            return new StaticConditionResult(exp, false, false);
          if (e.op == TOK.andAnd)
          {
-            if (!result)
-                return false;
+            if (!result.result)
+                return result;
          }
          else
          {
-            if (result)
-                return true;
+            if (result.result)
+                return result;
          }
-        result = evalStaticCondition(sc, exp, aae.e2, errors);
-        return !errors && result;
+        auto r3 = evalStaticCondition(sc, exp, aae.e2, errors);
+	result.e2 = r3;
+	result.result = r3.result;
+	if(errors)
+		result.result = false;
+        return result; // !errors && result;
      }

      if (e.op == TOK.question)
      {
          CondExp ce = cast(CondExp)e;
-        bool result = evalStaticCondition(sc, exp, ce.econd, 
errors);
+        auto result = evalStaticCondition(sc, exp, ce.econd, 
errors);
          if (errors)
-            return false;
-        Expression leg = result ? ce.e1 : ce.e2;
-        result = evalStaticCondition(sc, exp, leg, errors);
-        return !errors && result;
+            return new StaticConditionResult(exp, false, false);
+        Expression leg = result.result ? ce.e1 : ce.e2;
+        result.e1 = evalStaticCondition(sc, exp, leg, errors);
+	result.result = result.e1.result;
+	if(errors)
+		result.result = false;
+        return result;
      }

      uint nerrors = global.errors;
   -80,6 +133,8    bool evalStaticCondition(Scope* sc, Expression 
exp, Expression e, ref bool error
      sc = sc.startCTFE();
      sc.flags |= SCOPE.condition;

+    auto originalE = e;
+
      e = e.expressionSemantic(sc);
      e = resolveProperties(sc, e);

   -91,7 +146,7    bool evalStaticCondition(Scope* sc, Expression 
exp, Expression e, ref bool error
          e.type.toBasetype() == Type.terror)
      {
          errors = true;
-        return false;
+        return new StaticConditionResult(exp, false, false);
      }

      e = resolveAliasThis(sc, e);
   -100,17 +155,17    bool evalStaticCondition(Scope* sc, 
Expression exp, Expression e, ref bool error
      {
          exp.error("expression `%s` of type `%s` does not have a 
boolean value", exp.toChars(), e.type.toChars());
          errors = true;
-        return false;
+        return new StaticConditionResult(exp, false, false);
      }

      e = e.ctfeInterpret();

      if (e.isBool(true))
-        return true;
+        return new StaticConditionResult(originalE, true, true);
      else if (e.isBool(false))
-        return false;
+        return new StaticConditionResult(originalE, false, true);

      e.error("expression `%s` is not constant", e.toChars());
      errors = true;
-    return false;
+    return new StaticConditionResult(exp, false, false);
  }
Mar 04 2019
parent reply Adam D. Ruppe <destructionator gmail.com> writes:
Improved patch. Error message:

/home/me/test/ooooo.d(21): Error: template 
std.algorithm.sorting.sort cannot deduce function from argument 
types !()(FilterResult!(unaryFun, int[])), candidates are:
../generated/linux/release/64/../../../../../phobos/std/algori
hm/sorting.d(1855):        std.algorithm.sorting.sort(alias
ess = "a < b", SwapStrategy ss = SwapStrategy.unstable, 
Range)(Range r) if ((ss == SwapStrategy.unstable && 
(hasSwappableElements!Range || hasAssignableElements!Range) || ss 
!= SwapStrategy.unstable && hasAssignableElements!Range) && 
isRandomAccessRange!Range && hasSlicing!Range && hasLength!Range)
/home/me/test/ooooo.d(21):        
isRandomAccessRange!(FilterResult!(unaryFun, int[]))/* was false 
*/



See that it now gives it as an errorSupplemental instead of 
random printf spam. My biggest problem with it now is 1) there's 
only one supplemental info pointer... so for multiple 
non-matching candidates, only the last one actually shows extra 
info! and lesser concern, 2) formatting in even more complex cases


Regardless, here's the code:


diff --git a/src/dmd/cond.d b/src/dmd/cond.d
index 793cefc..ece927b 100644
--- a/src/dmd/cond.d
+++ b/src/dmd/cond.d
   -855,7 +855,7    extern (C++) final class StaticIfCondition : 
Condition

              import dmd.staticcond;
              bool errors;
-            bool result = evalStaticCondition(sc, exp, exp, 
errors);
+            bool result = evalStaticCondition(sc, exp, exp, 
errors).result;

              // Prevent repeated condition evaluation.
              // See: fail_compilation/fail7815.d
diff --git a/src/dmd/dtemplate.d b/src/dmd/dtemplate.d
index 933df8d..e9e5c1d 100644
--- a/src/dmd/dtemplate.d
+++ b/src/dmd/dtemplate.d
   -684,10 +684,11    extern (C++) final class 
TemplateDeclaration : ScopeDsymbol
          return protection;
      }

+    import dmd.staticcond;
      /****************************
       * Check to see if constraint is satisfied.
       */
-    extern (D) bool evaluateConstraint(TemplateInstance ti, 
Scope* sc, Scope* paramscope, Objects* dedargs, FuncDeclaration 
fd)
+    extern (D) StaticConditionResult 
evaluateConstraint(TemplateInstance ti, Scope* sc, Scope* 
paramscope, Objects* dedargs, FuncDeclaration fd)
      {
          /* Detect recursive attempts to instantiate this 
template declaration,
           * https://issues.dlang.org/show_bug.cgi?id=4072
   -713,7 +714,7    extern (C++) final class TemplateDeclaration 
: ScopeDsymbol
                  for (Scope* scx = sc; scx; scx = scx.enclosing)
                  {
                      if (scx == p.sc)
-                        return false;
+                        return new StaticConditionResult(null, 
false, false);
                  }
              }
              /* BUG: should also check for ref param differences
   -788,13 +789,13    extern (C++) final class 
TemplateDeclaration : ScopeDsymbol
          ti.inst = ti; // temporary instantiation to enable 
genIdent()
          scx.flags |= SCOPE.constraint;
          bool errors;
-        bool result = evalStaticCondition(scx, constraint, e, 
errors);
+        auto result = evalStaticCondition(scx, constraint, e, 
errors);
          ti.inst = null;
          ti.symtab = null;
          scx = scx.pop();
          previous = pr.prev; // unlink from threaded list
          if (errors)
-            return false;
+            return new StaticConditionResult(null, false, false);
          return result;
      }

   -833,7 +834,7    extern (C++) final class TemplateDeclaration 
: ScopeDsymbol
       *      dedtypes        deduced arguments
       * Return match level.
       */
-    extern (D) MATCH matchWithInstance(Scope* sc, 
TemplateInstance ti, Objects* dedtypes, Expressions* fargs, int 
flag)
+    extern (D) MatchWithSupplementalInformation 
matchWithInstance(Scope* sc, TemplateInstance ti, Objects* 
dedtypes, Expressions* fargs, int flag)
      {
          enum LOGM = 0;
          static if (LOGM)
   -848,11 +849,12    extern (C++) final class 
TemplateDeclaration : ScopeDsymbol
          }
          MATCH m;
          size_t dedtypes_dim = dedtypes.dim;
+	StaticConditionResult supplementalInformation;

          dedtypes.zero();

          if (errors)
-            return MATCH.nomatch;
+            return 
MatchWithSupplementalInformation(MATCH.nomatch);

          size_t parameters_dim = parameters.dim;
          int variadic = isVariadic() !is null;
   -864,7 +866,7    extern (C++) final class TemplateDeclaration 
: ScopeDsymbol
              {
                  printf(" no match: more arguments than 
parameters\n");
              }
-            return MATCH.nomatch;
+            return 
MatchWithSupplementalInformation(MATCH.nomatch);
          }

          assert(dedtypes_dim == parameters_dim);
   -970,7 +972,8    extern (C++) final class TemplateDeclaration 
: ScopeDsymbol
              }

              // TODO: dedtypes => ti.tiargs ?
-            if (!evaluateConstraint(ti, sc, paramscope, 
dedtypes, fd))
+	    supplementalInformation = evaluateConstraint(ti, sc, 
paramscope, dedtypes, fd);
+            if (!supplementalInformation.result)
                  goto Lnomatch;
          }

   -1016,7 +1019,7    extern (C++) final class 
TemplateDeclaration : ScopeDsymbol
          {
              printf("-TemplateDeclaration.matchWithInstance(this 
= %p, ti = %p) = %d\n", this, ti, m);
          }
-        return m;
+        return MatchWithSupplementalInformation(m);
      }

      /********************************************
   -1025,7 +1028,7    extern (C++) final class 
TemplateDeclaration : ScopeDsymbol
       *      match   this is at least as specialized as td2
       *      0       td2 is more specialized than this
       */
-    MATCH leastAsSpecialized(Scope* sc, TemplateDeclaration td2, 
Expressions* fargs)
+    MatchWithSupplementalInformation leastAsSpecialized(Scope* 
sc, TemplateDeclaration td2, Expressions* fargs)
      {
          enum LOG_LEASTAS = 0;
          static if (LOG_LEASTAS)
   -1061,8 +1064,8    extern (C++) final class 
TemplateDeclaration : ScopeDsymbol
          Objects dedtypes = Objects(td2.parameters.dim);

          // Attempt a type deduction
-        MATCH m = td2.matchWithInstance(sc, ti, &dedtypes, 
fargs, 1);
-        if (m > MATCH.nomatch)
+        auto m = td2.matchWithInstance(sc, ti, &dedtypes, fargs, 
1);
+        if (m.match > MATCH.nomatch)
          {
              /* A non-variadic template is more specialized than a
               * variadic one.
   -1082,7 +1085,12    extern (C++) final class 
TemplateDeclaration : ScopeDsymbol
          {
              printf("  doesn't match, so is not as 
specialized\n");
          }
-        return MATCH.nomatch;
+        return MatchWithSupplementalInformation(MATCH.nomatch);
+    }
+
+    struct MatchWithSupplementalInformation {
+	MATCH match;
+	StaticConditionResult supplementalInformation;
      }

      /*************************************************
   -1101,13 +1109,14    extern (C++) final class 
TemplateDeclaration : ScopeDsymbol
       *          bit 0-3     Match template parameters by 
inferred template arguments
       *          bit 4-7     Match template parameters by initial 
template arguments
       */
-    extern (D) MATCH 
deduceFunctionTemplateMatch(TemplateInstance ti, Scope* sc, ref 
FuncDeclaration fd, Type tthis, Expressions* fargs)
+    extern (D) MatchWithSupplementalInformation 
deduceFunctionTemplateMatch(TemplateInstance ti, Scope* sc, ref 
FuncDeclaration fd, Type tthis, Expressions* fargs)
      {
          size_t nfparams;
          size_t nfargs;
          size_t ntargs; // array size of tiargs
          size_t fptupindex = IDX_NOTFOUND;
          MATCH match = MATCH.exact;
+	StaticConditionResult supplementalInformation;
          MATCH matchTiargs = MATCH.exact;
          ParameterList fparameters; // function parameter list
          VarArg fvarargs; // function varargs
   -1142,7 +1151,7    extern (C++) final class 
TemplateDeclaration : ScopeDsymbol
          dedtypes.zero();

          if (errors || fd.errors)
-            return MATCH.nomatch;
+            return 
MatchWithSupplementalInformation(MATCH.nomatch);

          // Set up scope for parameters
          Scope* paramscope = scopeForTemplateParameters(ti,sc);
   -2003,8 +2012,10    extern (C++) final class 
TemplateDeclaration : ScopeDsymbol

          if (constraint)
          {
-            if (!evaluateConstraint(ti, sc, paramscope, dedargs, 
fd))
+	    supplementalInformation = evaluateConstraint(ti, sc, 
paramscope, dedargs, fd);
+            if (!supplementalInformation.result) {
                  goto Lnomatch;
+            }
          }

          version (none)
   -2018,18 +2029,19    extern (C++) final class 
TemplateDeclaration : ScopeDsymbol

          paramscope.pop();
          //printf("\tmatch %d\n", match);
-        return cast(MATCH)(match | (matchTiargs << 4));
+        return 
MatchWithSupplementalInformation(cast(MATCH)(match | (matchTiargs 
<< 4)));

      Lnomatch:
          paramscope.pop();
          //printf("\tnomatch\n");
-        return MATCH.nomatch;
+	//asm { int 3; }
+        return MatchWithSupplementalInformation(MATCH.nomatch, 
supplementalInformation);

      Lerror:
          // todo: for the future improvement
          paramscope.pop();
          //printf("\terror\n");
-        return MATCH.nomatch;
+        return MatchWithSupplementalInformation(MATCH.nomatch);
      }

      /**************************************************
   -2592,10 +2604,14    void functionResolve(Match* m, Dsymbol 
dstart, Loc loc, Scope* sc, Objects* tiar
              auto ti = new TemplateInstance(loc, td, tiargs);
              Objects dedtypes = Objects(td.parameters.dim);
              assert(td.semanticRun != PASS.init);
-            MATCH mta = td.matchWithInstance(sc, ti, &dedtypes, 
fargs, 0);
+            auto mta = td.matchWithInstance(sc, ti, &dedtypes, 
fargs, 0);
              //printf("matchWithInstance = %d\n", mta);
-            if (mta <= MATCH.nomatch || mta < ta_last)   // no 
match or less match
+            if (mta.match <= MATCH.nomatch || mta.match < 
ta_last)   // no match or less match
+            {
+	        if(pMessage && mta.supplementalInformation)
+		  *pMessage = mta.supplementalInformation.toChars();
                  return 0;
+            }

              ti.templateInstanceSemantic(sc, fargs);
              if (!ti.inst)               // if template failed to 
expand
   -2665,8 +2681,8    void functionResolve(Match* m, Dsymbol 
dstart, Loc loc, Scope* sc, Objects* tiar
              if (mfa < m.last)
                  return 0;

-            if (mta < ta_last) goto Ltd_best2;
-            if (mta > ta_last) goto Ltd2;
+            if (mta.match < ta_last) goto Ltd_best2;
+            if (mta.match > ta_last) goto Ltd2;

              if (mfa < m.last) goto Ltd_best2;
              if (mfa > m.last) goto Ltd2;
   -2686,7 +2702,7    void functionResolve(Match* m, Dsymbol 
dstart, Loc loc, Scope* sc, Objects* tiar
              td_best = td;
              ti_best = null;
              property = 0;   // (backward compatibility)
-            ta_last = mta;
+            ta_last = mta.match;
              m.last = mfa;
              m.lastf = fd;
              tthis_best = tthis_fd;
   -2708,12 +2724,18    void functionResolve(Match* m, Dsymbol 
dstart, Loc loc, Scope* sc, Objects* tiar
              ti.parent = td.parent;  // Maybe calculating valid 
'enclosing' is unnecessary.

              auto fd = f;
-            int x = td.deduceFunctionTemplateMatch(ti, sc, fd, 
tthis, fargs);
+            auto information = 
td.deduceFunctionTemplateMatch(ti, sc, fd, tthis, fargs);
+	    int x = information.match;
              MATCH mta = cast(MATCH)(x >> 4);
              MATCH mfa = cast(MATCH)(x & 0xF);
              //printf("match:t/f = %d/%d\n", mta, mfa);
              if (!fd || mfa == MATCH.nomatch)
+	    {
+	
+	        if(pMessage && information.supplementalInformation)
+		  *pMessage = information.supplementalInformation.toChars();
                  continue;
+            }

              Type tthis_fd = fd.needThis() ? tthis : null;

   -2743,8 +2765,8    void functionResolve(Match* m, Dsymbol 
dstart, Loc loc, Scope* sc, Objects* tiar
              if (td_best)
              {
                  // Disambiguate by picking the most specialized 
TemplateDeclaration
-                MATCH c1 = td.leastAsSpecialized(sc, td_best, 
fargs);
-                MATCH c2 = td_best.leastAsSpecialized(sc, td, 
fargs);
+                MATCH c1 = td.leastAsSpecialized(sc, td_best, 
fargs).match;
+                MATCH c2 = td_best.leastAsSpecialized(sc, td, 
fargs).match;
                  //printf("1: c1 = %d, c2 = %d\n", c1, c2);
                  if (c1 > c2) goto Ltd;
                  if (c1 < c2) goto Ltd_best;
   -6897,7 +6919,7    extern (C++) class TemplateInstance : 
ScopeDsymbol
              assert(tempdecl._scope);
              // Deduce tdtypes
              tdtypes.setDim(tempdecl.parameters.dim);
-            if (!tempdecl.matchWithInstance(sc, this, &tdtypes, 
fargs, 2))
+            if (!tempdecl.matchWithInstance(sc, this, &tdtypes, 
fargs, 2).match)
              {
                  error("incompatible arguments for template 
instantiation");
                  return false;
   -6952,7 +6974,7    extern (C++) class TemplateInstance : 
ScopeDsymbol
                  dedtypes.zero();
                  assert(td.semanticRun != PASS.init);

-                MATCH m = td.matchWithInstance(sc, this, 
&dedtypes, fargs, 0);
+                MATCH m = td.matchWithInstance(sc, this, 
&dedtypes, fargs, 0).match;
                  //printf("matchWithInstance = %d\n", m);
                  if (m <= MATCH.nomatch) // no match at all
                      return 0;
   -6961,8 +6983,8    extern (C++) class TemplateInstance : 
ScopeDsymbol

                  // Disambiguate by picking the most specialized 
TemplateDeclaration
                  {
-                MATCH c1 = td.leastAsSpecialized(sc, td_best, 
fargs);
-                MATCH c2 = td_best.leastAsSpecialized(sc, td, 
fargs);
+                MATCH c1 = td.leastAsSpecialized(sc, td_best, 
fargs).match;
+                MATCH c2 = td_best.leastAsSpecialized(sc, td, 
fargs).match;
                  //printf("c1 = %d, c2 = %d\n", c1, c2);
                  if (c1 > c2) goto Ltd;
                  if (c1 < c2) goto Ltd_best;
   -7182,7 +7204,7    extern (C++) class TemplateInstance : 
ScopeDsymbol
                              return 1;
                          }
                      }
-                    MATCH m = td.matchWithInstance(sc, this, 
&dedtypes, null, 0);
+                    MATCH m = td.matchWithInstance(sc, this, 
&dedtypes, null, 0).match;
                      if (m <= MATCH.nomatch)
                          return 0;
                  }
diff --git a/src/dmd/func.d b/src/dmd/func.d
index 4fce63a..ade0e95 100644
--- a/src/dmd/func.d
+++ b/src/dmd/func.d
   -2718,7 +2718,8    FuncDeclaration resolveFuncCall(const ref 
Loc loc, Scope* sc, Dsymbol s,

      Match m;
      m.last = MATCH.nomatch;
-    functionResolve(&m, s, loc, sc, tiargs, tthis, fargs, null);
+    const(char)* supplementalInformation;
+    functionResolve(&m, s, loc, sc, tiargs, tthis, fargs, 
&supplementalInformation);
      auto orig_s = s;

      if (m.last > MATCH.nomatch && m.lastf)
   -2777,6 +2778,9    FuncDeclaration resolveFuncCall(const ref 
Loc loc, Scope* sc, Dsymbol s,
                  tiargsBuf.peekString(), fargsBuf.peekString());

              printCandidates(loc, td);
+
+	    if(supplementalInformation)
+	    	.errorSupplemental(loc, "%s", supplementalInformation);
          }
          else if (od)
          {
diff --git a/src/dmd/semantic2.d b/src/dmd/semantic2.d
index 36aced3..92bab02 100644
--- a/src/dmd/semantic2.d
+++ b/src/dmd/semantic2.d
   -101,7 +101,7    private extern(C++) final class 
Semantic2Visitor : Visitor

          import dmd.staticcond;
          bool errors;
-        bool result = evalStaticCondition(sc, sa.exp, sa.exp, 
errors);
+        bool result = evalStaticCondition(sc, sa.exp, sa.exp, 
errors).result;
          sc = sc.pop();
          if (errors)
          {
diff --git a/src/dmd/staticcond.d b/src/dmd/staticcond.d
index 5521a37..0a05dc0 100644
--- a/src/dmd/staticcond.d
+++ b/src/dmd/staticcond.d
   -27,6 +27,59    import dmd.tokens;
  import dmd.utils;


+class StaticConditionResult {
+	this(Expression exp, bool result, bool wasEvaluated) {
+		this.exp = exp;
+		this.result = result;
+		this.wasEvaluated = wasEvaluated;
+	}
+
+	Expression exp; /// original expression, for error messages
+	bool result; /// final result
+	bool wasEvaluated; /// if this part was evaluated at all
+
+	StaticConditionResult e1; /// result on the one side, if done
+	StaticConditionResult e2; /// result on the other side, if done
+
+	const(char)* toChars() {
+		string s = this.toString();
+		s ~= "\n";
+		return s.ptr;
+	}
+
+	override string toString() {
+		import core.stdc.string;
+		string s;
+		if(e1) {
+			auto p = e1.toString();
+			if(p.length) {
+				if(s.length)
+					s ~= "\n";
+				s ~= p;
+			}
+		}
+
+		if(e2) {
+			auto p = e2.toString();
+			if(p.length) {
+				if(s.length)
+					s ~= "\n";
+				s ~= p;
+			}
+		}
+
+		if(!e1 && !e2) {
+			if(wasEvaluated && !result) {
+				auto c = exp.toChars();
+
+				s ~= c[0 .. strlen(c)];
+				s ~= "/* was false */";
+			}
+		}
+
+		return s;
+	}
+}

  /********************************************
   * Semantically analyze and then evaluate a static condition at 
compile time.
   -42,37 +95,48    import dmd.utils;
   *      true if evaluates to true
   */

-bool evalStaticCondition(Scope* sc, Expression exp, Expression 
e, ref bool errors)
+StaticConditionResult evalStaticCondition(Scope* sc, Expression 
exp, Expression e, ref bool errors)
  {
+//if(e.parens) printf("PARANS %s\n\n", e.toChars);
      if (e.op == TOK.andAnd || e.op == TOK.orOr)
      {
          LogicalExp aae = cast(LogicalExp)e;
-        bool result = evalStaticCondition(sc, exp, aae.e1, 
errors);
+	auto result = new StaticConditionResult(e, false, true);
+        auto r2 = evalStaticCondition(sc, exp, aae.e1, errors);
+	result.e1 = r2;
+	result.result = r2.result;
          if (errors)
-            return false;
+            return new StaticConditionResult(exp, false, false);
          if (e.op == TOK.andAnd)
          {
-            if (!result)
-                return false;
+            if (!result.result)
+                return result;
          }
          else
          {
-            if (result)
-                return true;
+            if (result.result)
+                return result;
          }
-        result = evalStaticCondition(sc, exp, aae.e2, errors);
-        return !errors && result;
+        auto r3 = evalStaticCondition(sc, exp, aae.e2, errors);
+	result.e2 = r3;
+	result.result = r3.result;
+	if(errors)
+		result.result = false;
+        return result; // !errors && result;
      }

      if (e.op == TOK.question)
      {
          CondExp ce = cast(CondExp)e;
-        bool result = evalStaticCondition(sc, exp, ce.econd, 
errors);
+        auto result = evalStaticCondition(sc, exp, ce.econd, 
errors);
          if (errors)
-            return false;
-        Expression leg = result ? ce.e1 : ce.e2;
-        result = evalStaticCondition(sc, exp, leg, errors);
-        return !errors && result;
+            return new StaticConditionResult(exp, false, false);
+        Expression leg = result.result ? ce.e1 : ce.e2;
+        result.e1 = evalStaticCondition(sc, exp, leg, errors);
+	result.result = result.e1.result;
+	if(errors)
+		result.result = false;
+        return result;
      }

      uint nerrors = global.errors;
   -80,6 +144,8    bool evalStaticCondition(Scope* sc, Expression 
exp, Expression e, ref bool error
      sc = sc.startCTFE();
      sc.flags |= SCOPE.condition;

+    auto originalE = e;
+
      e = e.expressionSemantic(sc);
      e = resolveProperties(sc, e);

   -91,7 +157,7    bool evalStaticCondition(Scope* sc, Expression 
exp, Expression e, ref bool error
          e.type.toBasetype() == Type.terror)
      {
          errors = true;
-        return false;
+        return new StaticConditionResult(exp, false, false);
      }

      e = resolveAliasThis(sc, e);
   -100,17 +166,17    bool evalStaticCondition(Scope* sc, 
Expression exp, Expression e, ref bool error
      {
          exp.error("expression `%s` of type `%s` does not have a 
boolean value", exp.toChars(), e.type.toChars());
          errors = true;
-        return false;
+        return new StaticConditionResult(exp, false, false);
      }

      e = e.ctfeInterpret();

      if (e.isBool(true))
-        return true;
+        return new StaticConditionResult(originalE, true, true);
      else if (e.isBool(false))
-        return false;
+        return new StaticConditionResult(originalE, false, true);

      e.error("expression `%s` is not constant", e.toChars());
      errors = true;
-    return false;
+    return new StaticConditionResult(exp, false, false);
  }
Mar 05 2019
parent reply Nicholas Wilson <iamthewilsonator hotmail.com> writes:
On Tuesday, 5 March 2019 at 14:08:12 UTC, Adam D. Ruppe wrote:

 Regardless, here's the code:


Please do this as a GH PR, its very hard to read (and not 
particularly useful) on the forum.

Thanks!
Mar 05 2019
parent Adam D. Ruppe <destructionator gmail.com> writes:
On Tuesday, 5 March 2019 at 14:32:59 UTC, Nicholas Wilson wrote:

 On Tuesday, 5 March 2019 at 14:08:12 UTC, Adam D. Ruppe wrote:

 Regardless, here's the code:


 Please do this as a GH PR, its very hard to read (and not 
 particularly useful) on the forum.


You can copy/paste it into git apply, but meh:

https://github.com/dlang/dmd/pull/9419


Though I feel like a PR is higher pressure than an emailed proof 
of concept. I personally am unlikely to finish the dmd process - 
the test suite is really annoying to me and the dmd code style 
clashes with my own - I just wanna prove that there's potential 
in this approach. I estimate it to be a ~6 hour job to finish the 
feature, so it isn't trivial, but it is definitely doable.
Mar 05 2019
prev sibling parent "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Tue, Mar 05, 2019 at 01:49:26AM +0000, Nicholas Wilson via Digitalmars-d
wrote:

 On Tuesday, 5 March 2019 at 01:18:05 UTC, H. S. Teoh wrote:


[...]

 But to take care of the most common case, all we have to do is to
 assume that sig constraints are of the form (A && B && C && ...).
 The compiler only needs to report which of these top level conjuncts
 failed.  If a sig constraint isn't of this form, then fallback to
 reporting the entire constraint as failed, i.e., treat it as the
 case (A) (single-argument conjunction).


 
 You don't actually get (A && B && C && ...) you get (A && (B && (C &&
 ...))) (at least I think thats what you get I haven't been able to
 confirm the exact AST) each of which could contain arbitrary
 conjugation, so you need to recurse and that hurts my head.


[...]

(A && (B && (... ))) is pretty standard AST structure for representing a
list of conjunctions.  I don't think we need to worry about non-linear
trees of &&'s, we just have to deal with the common case of
right-recursion in the AST, so just traverse down the right child of
each && node and the corresponding left subtrees would be the major
"clauses" of the sig constraint.  That's probably good enough for a
first stab at the problem.


T

-- 
Leather is waterproof.  Ever see a cow with an umbrella?
Mar 04 2019
prev sibling parent reply Johannes Pfau <nospam example.com> writes:
Am Mon, 04 Mar 2019 17:18:05 -0800 schrieb H. S. Teoh:


 ... the problem (from the perspective of issuing a nice error) is that
 you can arbitrarily compose logic which makes sorting the what from the
 chaff extremely difficult and that signal to noise is very important
 (ever used -verrors=spec ?). Believe me I tried, saw a suggestion by
 aliak in a Phobos PR thread and thought that would make things so much
 easier, and thus arose that DIP.


 
 Well, if you're looking for a perfect solution, then yes this will be
 very complicated and hairy to implement.
 
 But to take care of the most common case, all we have to do is to assume
 that sig constraints are of the form (A && B && C && ...).  The compiler
 only needs to report which of these top level conjuncts failed.  If a
 sig constraint isn't of this form, then fallback to reporting the entire
 constraint as failed, i.e., treat it as the case (A) (single-argument
 conjunction).
 
 It's not a perfect solution, but having this is already a lot better
 than the current pessimal state of things.  Your DIP represents an
 improvement over this most basic step, but IMO we should at least have
 this basic step first.  Don't let the perfect become the enemy of the
 good yet again.
 
 
 T


The DIP seems kind of intrusive to me. And Parsing a DNF form has one 
major drawback: For quite some time now we actually advised people to 
refactor their constraints into external helper functions, in order to 
have nicer ddoc. E.g. isDigest!X instead of isOutputRange!X && ... So 
now, we'd have to recommend the exact opposite. And both approaches are 
limited.

I think we have to take a step back here and see if we can't come up with 
a minimal composable solution, based on D's current features. So to 
analyze this problem:

We have a complex template constraint system, which is a superset of 
concepts. We can implement concepts in the library just fine (e.g. 
https://github.com/atilaneves/concepts). What we can't do is provide nice 
error messages.
But why is that? Well, all checking is done by CTFE D code, so with a 
CTFE library implementation of concepts, the compiler actually knows 
nothing about the concepts. So the only one being able to generate proper 
error messages is this CTFE library code. We don't we do that? Because we 
have no proper way to emit error messages! The pragma msg/static assert 
approaches all don't compose well with other D features 
(overloading, ...) and they won't produce nice error messages anyway. 
Probably better, but not nice.

So what if we simply introduce a bool __traits(constraintCheck, alias, 
condition, formatString) hook? Now we could do this:

struct InputRange
{
    T front;
    void popFront();
}

bool implements(T, Interface)
{
    bool result = true;
    foreach (member; Interface)
    {
        if (!hasMember(T, member))
            result &= __traits(constraintCheck, T, false, "Type %S does 
not implement interface member " ~ niceFormatting(...));
    }
    return result;
}

void testRange(R, T)(R range) if (implements!(R, InputRange!T))
{
    
}

void testRange(R, T)(R range) if (implements!(R, OutputRange!T))
{
    
}
====================
test.d:6:8: Error: template »testRange« cannot deduce function from 
argument types »!(Foo, Bar)« candidates are:

test.d:1:6: Note: »(R, T)(R range) if (implements!(R, InputRange!T))«:
     R: Type 'Foo' does not implement interface member 'Bar   
     InputRange.front'.
     R: Type 'Foo' does not implement interface member 'popFront()'.

     testRange!(Foo, Bar);
                 ^


test.d:4:6: Note: »(R, T)(R range) if (implements!(R, OutputRange!T))«:
     R: Type 'Foo' does not implement interface member 'put(Bar)'.

     testRange!(Foo, Bar);
                 ^


void testOdd(uint n)() if (__traits(constraintCheck, n, n % 2 == 1, "Need 
an odd integer."))
====================
test.d:6:8: Error: template »testOdd« cannot deduce function from 
argument types »!(2)« candidates are:

test.d:1:6: Note: »testOdd(uint n)«: n: Need an odd integer.
     testOdd(2);
             ^

I think this is really all we need to have to implement a replacement for 
concepts with perfectly nice error messages. We may have to put some 
thought into the exact design of __traits(constraintCheck), but I think 
it's perfectly possible. And if we the provide a implementsConcept!T in 
phobos, we should be fine. Even DDOC generators can largely benefit from 
this without any changes, but they could also recognize the 
`implementsConcept` name and provide specially formatted docs. An this 
approach will preserve flexibility of the current system while providing 
nice error messages for more cases than concepts could (see testOdd 
above).

Open questions:

* What to do if there are multiple overloads. Print diagnostics for all?
* If multiple __traits(constraintCheck) fail, do we print all?
* If multiple failed checks reference same alias we should merge the  
  diagnostic source code location info for all these?
* Do we allow the alias to refer to a member of some parameter type? If 
  so, where will diagnostics point at: the parameter or the definition of
  the member?: 

test.d:1:6: Note: »(R, T)(R range) if (implements!(R, OutputRange!T))«:
     R: interface member 'Foo.put(Bar)' is not  safe.

     test.d:2.4 void put(T t)
                            ^
* What do we want to allow in the format string? I think we need at least 
  the original type name in user context, though maybe we can also get   
  that in some other way. Maybe the original parameter name is useful?

* We should also consider what locations to print (Is the location of the 
original overload definition useful?)


-- 
Johannes
Mar 07 2019
next sibling parent Meta <jared771 gmail.com> writes:
On Thursday, 7 March 2019 at 22:09:29 UTC, Johannes Pfau wrote:

 * What to do if there are multiple overloads. Print diagnostics 
 for all?


It's not ideal, but a -constraintcheck compiler switch would mean 
that these messages are only printed when a user explicitly 
requests it (and for bonus points: -constraintcheck=<list of 
symbols> to only check certain symbols).


 * If multiple __traits(constraintCheck) fail, do we print all?


I don't see any other way you could do it.
Mar 07 2019
prev sibling parent reply Nicholas Wilson <iamthewilsonator hotmail.com> writes:
On Thursday, 7 March 2019 at 22:09:29 UTC, Johannes Pfau wrote:

 The DIP seems kind of intrusive to me. And Parsing a DNF


The DIP formalises a constraint in a _CNF_, not DNF.


 form has one major drawback: For quite some time now we 
 actually advised people to refactor their constraints into 
 external helper functions, in order to have nicer ddoc. E.g. 
 isDigest!X instead of isOutputRange!X && ...


Yes, but this is for a single concept for a single parameter. 
Real constraints deal with multiple parameters and conjunctions 
and disjunction of concepts, hence the CNF formalisation. The 
rest of what you describe is great, but completely orthogonal.

I have made this comment before: 
https://github.com/dlang/DIPs/pull/131#issuecomment-416555780


 So now, we'd have to recommend the exact opposite.


This follows only for a single parameter.
Mar 07 2019
parent reply Johannes Pfau <nospam example.com> writes:
Am Fri, 08 Mar 2019 05:46:01 +0000 schrieb Nicholas Wilson:


 On Thursday, 7 March 2019 at 22:09:29 UTC, Johannes Pfau wrote:

 The DIP seems kind of intrusive to me. And Parsing a DNF


 
 The DIP formalises a constraint in a _CNF_, not DNF.


You're right of course, I guess I was too tired when I wrote that.


 form has one major drawback: For quite some time now we actually
 advised people to refactor their constraints into external helper
 functions, in order to have nicer ddoc. E.g.
 isDigest!X instead of isOutputRange!X && ...


 
 Yes, but this is for a single concept for a single parameter. Real
 constraints deal with multiple parameters and conjunctions and
 disjunction of concepts, hence the CNF formalisation. The rest of what
 you describe is great, but completely orthogonal.
 
 I have made this comment before:
 https://github.com/dlang/DIPs/pull/131#issuecomment-416555780
 

 So now, we'd have to recommend the exact opposite.


 
 This follows only for a single parameter.


OK, I see. For multiple parameters a CNF form is indeed the natural 
description. However, the DIP probably wouldn't be needed with 
traits(constraintsCheck) as multiple error reporting would also work with 
a CNF of implements!T or other constraints. One thing a simple 
traits(constraintsCheck) can probably not handle easily is pointing out 
which of the implements!T calls caused the errors, though if we detect CNF 
in the compiler that could work. Really difficult are more complicated 
constructs such as (isSomething!(A, B) || isSomething!(A, C)). Here 
traits(constraintsCheck) would produce all messages for both checks if 
both fail.

-- 
Johannes
Mar 08 2019
parent Nicholas Wilson <iamthewilsonator hotmail.com> writes:
On Friday, 8 March 2019 at 08:19:41 UTC, Johannes Pfau wrote:

 OK, I see. For multiple parameters a CNF form is indeed the 
 natural description. However, the DIP probably wouldn't be 
 needed with traits(constraintsCheck) as multiple error 
 reporting would also work with a CNF of implements!T or other 
 constraints. One thing a simple traits(constraintsCheck) can 
 probably not handle easily is pointing out which of the 
 implements!T calls caused the errors,


That is why this is orthogonal to the DIP, nothing stops us doing 
both.


 though if we detect CNF in the compiler that could work. Really 
 difficult are more complicated constructs such as 
 (isSomething!(A, B) || isSomething!(A, C)).


No, _really_ difficult constraints are recursive on variable 
arguments

ptrdiff_t countUntil(alias pred = "a == b", R, Rs...)(R haystack, 
Rs needles)
if (isForwardRange!R
     && Rs.length > 0
     && isForwardRange!(Rs[0]) == isInputRange!(Rs[0])
     && is(typeof(startsWith!pred(haystack, needles[0])))
     && (Rs.length == 1
     || is(typeof(countUntil!pred(haystack, needles[1 .. $])))))

https://github.com/dlang/phobos/blob/master/std/algorithm/searching.d#L768


 Here traits(constraintsCheck) would produce all messages for 
 both checks if both fail.


List the output for a predicate that is invalid for the second 
needle, or a second needle that is an input range but not a 
forward range or a second needle that is not an input range but 
fails for some other reason.

In the first case you're in an is expression and typeof and the 
constraint comes from the failed constraint of the recursion of 
the current template that depends on another template (startsWith)

The second still doesn't enable expressing the fact the 
countUntil requires needles to either be elements or forward 
ranges, which is not at all obvious from the expression used to 
encode the sub-constraint.

The third you _have to_ use the CNF property or else you'll emit 
errors for unrelated things.
Mar 08 2019
prev sibling parent "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Mon, Mar 04, 2019 at 11:03:28PM +0000, Adam D. Ruppe via Digitalmars-d wrote:

 On Monday, 4 March 2019 at 15:47:12 UTC, H. S. Teoh wrote:

 This is why I'm starting to think sig constraints are not the genius
 idea they first appeared to be.


 
 If the implementation actually told you which individual pieces failed
 (and perhaps even why), I don't think we'd be worried about this.


[....]

 Imagine what we'd say if the error message was not:


[...]

 but rather
 
 
 
 ooooo.d(4): Error: template std.algorithm.sorting.sort cannot deduce
 function from argument types
    !()(FilterResult!(unaryFun, int[])), candidates are:
 /home/me/d/dmd2/linux/bin32/../../src/phobos/std/algorithm/sorting.d(1847):
    std.algorithm.sorting.sort([collapsed])(Range r)
       if (
         // passed
         (ss == SwapStrategy.unstable &&
         // passed
         (hasSwappableElements!Range || hasAssignableElements!Range) ||
          // short-circuited
         ss != SwapStrategy.unstable &&
 asAssignableElements!Range)
         // ******* FAILED ***********
         && isRandomAccessRange!Range
         // short-circuited, but would fail
         && hasSlicing!Range && hasLength!Range
       )
 
 
 That tells you everything you need to know.


Yes, this is indeed a big improvement over what we currently have.  It
may not be perfect, but it goes a long way from where we are right now.
Also, it should be relatively straightforward to implement, and doesn't
break any existing code, etc..

Where's the PR for this??? I want it NAO! :-D


T

-- 
A mathematician is a device for turning coffee into theorems. -- P. Erdos
Mar 04 2019
prev sibling next sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 3/4/19 10:47 AM, H. S. Teoh wrote:

 On Mon, Mar 04, 2019 at 02:50:11PM +0000, Adam D. Ruppe via Digitalmars-d
wrote:
 [...]

 How many template constraint messages would be nice if it just said
 "missing popFront" instead of 40 lines of overload failed spam?


 [...]
 
 This is why I'm starting to think sig constraints are not the genius
 idea they first appeared to be.


A while ago, someone in this forum (or Walter or even myself) had an 
idea on how to associate error messages with template constraints. 
Someone pointed out an in-the-works DIP about it. Couldn't find it, 
where is it?
Mar 05 2019
parent Nicholas Wilson <iamthewilsonator hotmail.com> writes:
On Tuesday, 5 March 2019 at 11:49:09 UTC, Andrei Alexandrescu 
wrote:

 On 3/4/19 10:47 AM, H. S. Teoh wrote:

 On Mon, Mar 04, 2019 at 02:50:11PM +0000, Adam D. Ruppe via 
 Digitalmars-d wrote:
 [...]

 How many template constraint messages would be nice if it 
 just said
 "missing popFront" instead of 40 lines of overload failed 
 spam?


 [...]
 
 This is why I'm starting to think sig constraints are not the 
 genius
 idea they first appeared to be.


 A while ago, someone in this forum (or Walter or even myself) 
 had an idea on how to associate error messages with template 
 constraints. Someone pointed out an in-the-works DIP about it. 
 Couldn't find it, where is it?


Try looking ;)

https://github.com/dlang/DIPs/pull/131
Mar 05 2019
prev sibling parent reply FeepingCreature <feepingcreature gmail.com> writes:
On Monday, 4 March 2019 at 15:47:12 UTC, H. S. Teoh wrote:

 On Mon, Mar 04, 2019 at 02:50:11PM +0000, Adam D. Ruppe via 
 Digitalmars-d wrote: [...]

 How many template constraint messages would be nice if it just 
 said "missing popFront" instead of 40 lines of overload failed 
 spam?


 [...]

 This is why I'm starting to think sig constraints are not the 
 genius idea they first appeared to be. The basic problem is 
 that when you write a sig constraint, you're basically saying 
 "I only accept template arguments if they don't cause an error, 
 otherwise it's not my problem and somebody else can pick up the 
 tab", whereas the essence of user-friendly error-reporting is 
 "I accept everything that looks like it ought to work, and if 
 something breaks, I'll tell you why it didn't work".


Personally, I think the problem comes down to this: we have no 
way of propagating errors from  inside template constraints to 
the user.

I think the way to go is instead of returning `false`, have 
__traits(compiles) (which is really the core source of these 
difficulties) return an "error object" that evaluates to false, 
but also encodes additional information about the problem, 
recursively.

Propagate this through short-circuiting - ErrorObject && bla = 
true, bla && ErrorObject = ErrorObject, etc.

The compiler can then pick up on the fact that an error object 
was the reason why a template constraint failed to give a highly 
informative error message.

For instance:

struct Foo
{
   void bar() { }
}

enum hasFooCall(T) = __traits(compiles, T.init.foo());

void callFoo(T)(T t)
if (hasFooCall!T)
{
   t.foo();
}

callFoo!Foo(Foo.init);

then errors with

Error: template onlineapp.callFoo cannot deduce function from 
argument types !()(Foo), candidates are:
   onlineapp.callFoo(T)(T t) if (hasFooCall!T)
   Failed because:
     hasFooCall(Foo): no property foo for type Foo
Mar 06 2019
next sibling parent Olivier FAURE <couteaubleu gmail.com> writes:
On Wednesday, 6 March 2019 at 12:48:08 UTC, FeepingCreature wrote:

 I think the way to go is instead of returning `false`, have 
 __traits(compiles) (which is really the core source of these 
 difficulties) return an "error object" that evaluates to false, 
 but also encodes additional information about the problem, 
 recursively.


YES!

I'd go further and say, use error objects in any trait that 
returns a boolean, and in is expressions.
Mar 06 2019
prev sibling next sibling parent Johannes Pfau <nospam example.com> writes:
Am Wed, 06 Mar 2019 12:48:08 +0000 schrieb FeepingCreature:


 Personally, I think the problem comes down to this: we have no way of
 propagating errors from  inside template constraints to the user.
 
 I think the way to go is instead of returning `false`, have
 __traits(compiles) (which is really the core source of these
 difficulties) return an "error object" that evaluates to false, but also
 encodes additional information about the problem, recursively.
 
 Propagate this through short-circuiting - ErrorObject && bla = true, bla
 && ErrorObject = ErrorObject, etc.
 
 The compiler can then pick up on the fact that an error object was the
 reason why a template constraint failed to give a highly informative
 error message.


I see we both had the same idea! Instead of special casing 
traits(compiles) and making the compiler deduce an error message I'd let 
the CTFE library code do that, see my post about 
__traits(constraintCheck). Also we should allow reporting multiple errors 
at once, instead of one at a time (e.g. a Type not implementing front and 
popFront for input ranges should report both problems at once).

But something like this is in my opinion the best way to go.


-- 
Johannes
Mar 07 2019
prev sibling parent reply Nicholas Wilson <iamthewilsonator hotmail.com> writes:
On Wednesday, 6 March 2019 at 12:48:08 UTC, FeepingCreature wrote:

 Personally, I think the problem comes down to this: we have no 
 way of propagating errors from  inside template constraints to 
 the user.

 I think the way to go is instead of returning `false`, have 
 __traits(compiles) (which is really the core source of these 
 difficulties)


I find `is` expressions to be more to blame, but the underlying 
reason is the same: the error are gagged. This is good most of 
the time (because otherwise it would be like always using 
verrors=spec), but it misses the small fraction of the time when 
that is extremely useful.


 return an "error object" that evaluates to false, but also 
 encodes additional information about the problem, recursively.

 Propagate this through short-circuiting - ErrorObject && bla = 
 true, bla && ErrorObject = ErrorObject, etc.

 The compiler can then pick up on the fact that an error object 
 was the reason why a template constraint failed to give a 
 highly informative error message.

 For instance:
 *snip*


This works nicely for __traits compiles, I'm not so sure about is 
expressions (though I'd love to be shown otherwise).
Mar 07 2019
parent reply FeepingCreature <feepingcreature gmail.com> writes:
On Friday, 8 March 2019 at 06:09:30 UTC, Nicholas Wilson wrote:

 This works nicely for __traits compiles, I'm not so sure about 
 is expressions (though I'd love to be shown otherwise).


I'm not sure what the problem with `is` is? `is` is also designed 
to return false on compile failure and true on success, so 
replacing its return value with an ErrorObject doesn't seem like 
it *should* break things.
Mar 07 2019
parent reply Nicholas Wilson <iamthewilsonator hotmail.com> writes:
On Friday, 8 March 2019 at 06:34:00 UTC, FeepingCreature wrote:

 On Friday, 8 March 2019 at 06:09:30 UTC, Nicholas Wilson wrote:

 This works nicely for __traits compiles, I'm not so sure about 
 is expressions (though I'd love to be shown otherwise).


 I'm not sure what the problem with `is` is? `is` is also 
 designed to return false on compile failure and true on 
 success, so replacing its return value with an ErrorObject 
 doesn't seem like it *should* break things.


But it also returns false if the expression is false

`is(T==int)` could return false because T is an error AST node, 
because T is undefined in the context or because T is not `int`. 
I'm missing how you would handle the latter case.
Mar 07 2019
parent FeepingCreature <feepingcreature gmail.com> writes:
On Friday, 8 March 2019 at 06:45:24 UTC, Nicholas Wilson wrote:

 But it also returns false if the expression is false

 `is(T==int)` could return false because T is an error AST node, 
 because T is undefined in the context or because T is not 
 `int`. I'm missing how you would handle the latter case.


Ah.

I guess in that case, since this is a compiletime expression and 
can change its type at will, you could simply just return `false`.
Mar 07 2019
prev sibling next sibling parent "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Mon, Mar 04, 2019 at 02:50:11PM +0000, Adam D. Ruppe via Digitalmars-d wrote:

 One of the reddit comments posted this link:
 
 https://elm-lang.org/blog/compiler-errors-for-humans


[...]

P.S. One point I disagree with in that article, though, is color.  I
find colors very tiresome on the eyes and distracting from focusing on
the code. Worse yet, they are often conflicting with the default
background colors I set on my terminal.  So if we were to do colors
(e.g. in recent dmd releases) I'd insist that there be an option to turn
it off.

I'm aware that my opinion is in the vast minority, though.


T

-- 
What are you when you run out of Monet? Baroque.
Mar 04 2019
prev sibling parent reply Jon Degenhardt <jond noreply.com> writes:
On Monday, 4 March 2019 at 14:50:11 UTC, Adam D. Ruppe wrote:

 One of the reddit comments posted this link:

 https://elm-lang.org/blog/compiler-errors-for-humans

 Wow, those error messages are MUCH better than D has. Want my 
 opinion on the best thing D should change for productivity? 
 Completely overhaul the error messages.


Another relevant blog post, this one about error message 
improvements in GCC 9: 
https://developers.redhat.com/blog/2019/03/08/usability-improvements-in-gcc-9/.

Not in the same league as what is described for Elm, but there 
may be useful ideas.
Mar 11 2019
next sibling parent reply Adam D. Ruppe <destructionator gmail.com> writes:
On Monday, 11 March 2019 at 23:41:13 UTC, Jon Degenhardt wrote:

 Not in the same league as what is described for Elm, but there 
 may be useful ideas.


Still very nice, much better than dmd has. Look, they use color 
to highlight relevant information instead of pointless syntax!

I'm so jealous I am seriously considering forking the compiler 
just to improve my own productivity instead of waiting on 
upstream to take error messages seriously.
Mar 11 2019
parent reply Nicholas Wilson <iamthewilsonator hotmail.com> writes:
On Tuesday, 12 March 2019 at 00:07:19 UTC, Adam D. Ruppe wrote:

 I am seriously considering forking the compiler just to improve 
 my own productivity instead of waiting on upstream to take 
 error messages seriously.


Please don't, we'd love to have those patches!

I'm a bit busy at the moment, but I've added better error 
messages to the agenda for dconf. So once we have some vision and 
direction we should be ready to make some serious progress, one 
major thing I think will help is to have tools to auto-update the 
test suite, I hate how over prescriptive it is.
Mar 11 2019
parent reply Seb <seb wilzba.ch> writes:
On Tuesday, 12 March 2019 at 00:40:32 UTC, Nicholas Wilson wrote:

 one major thing I think will help is to have tools to 
 auto-update the test suite, I hate how over prescriptive it is.


./test/run.d AUTO_UPDATE=1

(works with the Makefile too)
Mar 11 2019
parent reply Nicholas Wilson <iamthewilsonator hotmail.com> writes:
On Tuesday, 12 March 2019 at 06:33:55 UTC, Seb wrote:

 On Tuesday, 12 March 2019 at 00:40:32 UTC, Nicholas Wilson 
 wrote:

 one major thing I think will help is to have tools to 
 auto-update the test suite, I hate how over prescriptive it is.


 ./test/run.d AUTO_UPDATE=1

 (works with the Makefile too)


Thanks, could you add that to the dlang bot's message? I'm going 
to forget that otherwise.
Mar 11 2019
parent Seb <seb wilzba.ch> writes:
On Tuesday, 12 March 2019 at 06:50:45 UTC, Nicholas Wilson wrote:

 On Tuesday, 12 March 2019 at 06:33:55 UTC, Seb wrote:

 On Tuesday, 12 March 2019 at 00:40:32 UTC, Nicholas Wilson 
 wrote:

 one major thing I think will help is to have tools to 
 auto-update the test suite, I hate how over prescriptive it 
 is.


 ./test/run.d AUTO_UPDATE=1

 (works with the Makefile too)


 Thanks, could you add that to the dlang bot's message? I'm 
 going to forget that otherwise.


No, we can't add the entire README to every message.
Just remember to look here (the testsuite's README):

https://github.com/dlang/dmd/blob/master/test/README.md#automatically-update-the-test_output-segments
Mar 11 2019
prev sibling parent Olivier FAURE <couteaubleu gmail.com> writes:
On Monday, 11 March 2019 at 23:41:13 UTC, Jon Degenhardt wrote:

 Not in the same league as what is described for Elm, but there 
 may be useful ideas.


Oh wow, these are good. I might actually switch back to GCC for 
my C++ projects. I still remember, a few years ago, when GCC 
diagnostics didn't even include line contents, only error lines.
Mar 11 2019