View mode: basic / threaded / horizontal-split · Log in · Help
January 02, 2013
Proposal: prettify template constraints failure messages
The proposal is to extend template constraint from:

if(<expr>)
 to
if(<expr>; <message-expr>)

Where the <message-expr> is CTFE-able expression with type string.

The message-expr yields a hint string for compiler to show when 
instantiation failed instead of full expression that failed. It has to 
be CTFE-able even if template instantiation never actually fails.

The original full expression verbose behavior can be restored if desired 
with "verbose" compiler switch. "Verbose template instantiation failure 
switch" can be later be further enhanced to detect which sub-expressions 
are false for each candidate template.


Now how it should look like in messages.

Taking a motivating example from pull
https://github.com/D-Programming-Language/phobos/pull/1047
(though all of you have certainly seen far, far worse):

bug.d(11): Error: template std.range.chain does not match any function 
template declaration. Candidates are:
std/range.d(2018):        std.range.chain(Ranges...)(Ranges rs) if 
(Ranges.length > 0 && allSatisfy!(isInputRange, staticMap!(Unqual, 
Ranges)) && !is(CommonType!(staticMap!(ElementType, staticMap!(Unqual, 
Ranges))) == void))
bug.d(11): Error: template std.range.chain(Ranges...)(Ranges rs) if 
(Ranges.length > 0 && allSatisfy!(isInputRange, staticMap!(Unqual, 
Ranges)) && !is(CommonType!(staticMap!(ElementType, staticMap!(Unqual, 
Ranges))) == void)) cannot deduce template function from argument types 
!()(MapResult!(__lambda2, Foo[]),string)

First there is another sub-enhancement to remove constraints from the 
last message
Error: template XYZ <constraint *again* > cannot deduce ... from

as listing of candidates already shown them all.

Second and the actual point applied (message can be improved but in any 
case it should accurately & concisely describe the constraint):

std/range.d(2018):        std.range.chain(Ranges...)(Ranges rs) accepts: 
Any number of Input Ranges all having a common type among types of their 
elements.
bug.d(11): Error: template std.range.chain(Ranges...)(Ranges rs)
 cannot deduce template function from argument types 
!()(MapResult!(__lambda2, Foo[]),string)

where "accepts:" is compiler generated prefix and "Any number..." is a 
hint message.

What do you think?

-- 
Dmitry Olshansky
January 02, 2013
Re: Proposal: prettify template constraints failure messages
On Thursday, January 03, 2013 01:25:17 Dmitry Olshansky wrote:
> What do you think?

Hmmm. Typically the thing that you really want to know is which portions were 
true and which were false, and this doesn't help with that at all. What it 
does is make the errors more understandable for newbies, which is good. But 
for those of use who know more, they'd actually be annoying I think, because 
what I really want to know is what the actual constraint was so that I can 
figure out what's wrong. But I'm likely to look at the source for that anyway, 
since it's far more legible there than it is the error message.

So, this could be a good idea in that it could make the errors more 
understandable for newbies, but it still really doesn't help tell you what's 
wrong, which is what you really want to know. It just tries to translate the 
constraint into English. So, I don't know.

I'm not opposed to the idea, but I'm not exactly sold on it either. Certainly, 
I don't think that it would benefit me, personally, at all, but it may be worth 
it for newbies.

- Jonathan M Davis
January 02, 2013
Re: Proposal: prettify template constraints failure messages
On Wednesday, 2 January 2013 at 21:43:31 UTC, Jonathan M Davis 
wrote:
> On Thursday, January 03, 2013 01:25:17 Dmitry Olshansky wrote:
>> What do you think?
>
> Hmmm. Typically the thing that you really want to know is which 
> portions were
> true and which were false, and this doesn't help with that at 
> all. What it
> does is make the errors more understandable for newbies, which 
> is good. But
> for those of use who know more, they'd actually be annoying I 
> think, because
> what I really want to know is what the actual constraint was so 
> that I can
> figure out what's wrong. But I'm likely to look at the source 
> for that anyway,
> since it's far more legible there than it is the error message.
>
> So, this could be a good idea in that it could make the errors 
> more
> understandable for newbies, but it still really doesn't help 
> tell you what's
> wrong, which is what you really want to know. It just tries to 
> translate the
> constraint into English. So, I don't know.
>
> I'm not opposed to the idea, but I'm not exactly sold on it 
> either. Certainly,
> I don't think that it would benefit me, personally, at all, but 
> it may be worth
> it for newbies.
>
> - Jonathan M Davis

Well, to be honest, some of our overloads are really 
*complicated*. I swear that for some of them, even understanding 
what the constraint are and why is a huge headache. Having some 
english in there can't hurt, IMO. That said, judicious use of 
traits (even private traits) can achieve the same goal, and 
reduce the complexity.

----
That said, I think another field of improvement we should 
concentrate in phobos is to have a single public entry point, and 
then privately forward to private specialized overloads.

Users really shouldn't have to deal with functions whose 
restraints are things such as "input range, but not random 
access", or "random access with length, but not sliceable" or 
some of our other *6* liners. It gets especially bad when the 
compiler lists all 7 of our (implementation defined) overloads. 
Nobody needs to see that.

As a end user, I'd really want to see a single function with 
"isForwardRange!R". Period. The fact that the implementation 
differs depending on all that *crap* really isn't my problem and 
shouldn't show up in my interface >:(
January 02, 2013
Re: Proposal: prettify template constraints failure messages
On Wednesday, January 02, 2013 23:02:12 monarch_dodra wrote:
> That said, I think another field of improvement we should
> concentrate in phobos is to have a single public entry point, and
> then privately forward to private specialized overloads.
> 
> Users really shouldn't have to deal with functions whose
> restraints are things such as "input range, but not random
> access", or "random access with length, but not sliceable" or
> some of our other *6* liners. It gets especially bad when the
> compiler lists all 7 of our (implementation defined) overloads.
> Nobody needs to see that.
> 
> As a end user, I'd really want to see a single function with
> "isForwardRange!R". Period. The fact that the implementation
> differs depending on all that *crap* really isn't my problem and
> shouldn't show up in my interface >:(

I think that this is sensible. I think that I haven't been doing it primarily 
because of how some of the functions are laid out in the source file 
(particularly with regards to the unit tests). Some of them will still need to 
be separate though for documentation purposes (or need version(StdDdoc) blocks 
which provide documentation if they're merged into a single function or 
template). find would be a prime example of this.

There's also the question of how to combine funcitons. There are 3 ways that I 
can think of off the top of my head:

1. Put them in a single function with many static ifs (not terribly clean if 
the functions were big enough that you split them apart in the first place).

2. Use a public function which calls a private impl function, which 
potentially creates unnecessary overhead (especially without -inline) but 
allows you to keep the separate functions more or less as they are now 
internally.

3. Use a template with the simplified template constraint on it with inner 
templated functions for each of the overloads.

I've generally though of doing #3 (as I think that we've done that in some 
cases already), but it forces you to place the unit tests farther from the 
functions, which I don't like. You seem to be suggesting #2, and that would 
probably be a better approach because of that, though it does bug me to create 
a useless outer function just for a template constraint.

- Jonathan M Davis
January 04, 2013
Re: Proposal: prettify template constraints failure messages
I'm reposting message from Don Clugston. Somehow it's not on the list 
and reply-to email address looks inaccessible.

On 02/01/13 22:25, Dmitry Olshansky wrote:
> The proposal is to extend template constraint from:
>
> if(<expr>)
>   to
> if(<expr>; <message-expr>)
>
> Where the <message-expr> is CTFE-able expression with type string.
>
> The message-expr yields a hint string for compiler to show when
> instantiation failed instead of full expression that failed. It has to
> be CTFE-able even if template instantiation never actually fails.
>
> The original full expression verbose behavior can be restored if desired
> with "verbose" compiler switch. "Verbose template instantiation failure
> switch" can be later be further enhanced to detect which sub-expressions
> are false for each candidate template.

I think we should just introduce an "else" or "default" constraint. For 
example:

void chain(Ranges...)(Ranges rs) default
{
  static assert("std.range.chain requires: Any number of Input Ranges 
all having a common type among types of their elements.");
}


ie, the default is used only if all template constraints fail.

/end of message

Which could be a more coherent way to solve it. The problem I see is 
that messages are disconnected from signatures of failed templates (that 
can change later and then message is easily out sync).

Either way what about just removing the constraints on the second error 
message here:

bug.d(11): Error: template std.range.chain does not match any function 
template declaration. Candidates are:
std/range.d(2018):        std.range.chain(Ranges...)(Ranges rs) if 
(Ranges.length > 0 && allSatisfy!(isInputRange, staticMap!(Unqual, 
Ranges)) && !is(CommonType!(staticMap!(ElementType, staticMap!(Unqual, 
Ranges))) == void))

That was the first, it list candidates. It's relatively new thing, nice 
to have it.

bug.d(11): Error: template std.range.chain(Ranges...)(Ranges rs) if 
(Ranges.length > 0 && allSatisfy!(isInputRange, staticMap!(Unqual, 
Ranges)) && !is(CommonType!(staticMap!(ElementType, staticMap!(Unqual, 
Ranges))) == void)) cannot deduce template function from argument types 
!()(MapResult!(__lambda2, Foo[]),string)

The second need not to realist constraints here, they are obvious from 
above, what needed is the last line - template arguments. So just make it:

bug.d(11): Error: template std.range.chain(Ranges...)(Ranges rs)  cannot 
deduce template function from argument types !()(MapResult!(__lambda2, 
Foo[]),string)

Bugzilla worthy?

-- 
Dmitry Olshansky
January 04, 2013
Re: Proposal: prettify template constraints failure messages
On Friday, January 04, 2013 18:16:44 Dmitry Olshansky wrote:
> I think we should just introduce an "else" or "default" constraint. For
> example:
> 
> void chain(Ranges...)(Ranges rs) default
> {
> static assert("std.range.chain requires: Any number of Input Ranges
> all having a common type among types of their elements.");
> }
> 
> 
> ie, the default is used only if all template constraints fail.
> 
> /end of message
> 
> Which could be a more coherent way to solve it. The problem I see is
> that messages are disconnected from signatures of failed templates (that
> can change later and then message is easily out sync).

Doing that wouldn't be all that different from using your suggestion with a 
single point of entry (e.g. using an outer template or wrapper function with a 
simplified template constraint with the separate functions having the more 
detailed constraints required to separate them). And with your suggestion, you 
could give each candidate an informative message if you wanted multiple points 
of entry (though I'd favor listing the message in addition to the constraint 
rather than instead of like you proposed).

> Either way what about just removing the constraints on the second error
> message here:
> 
> bug.d(11): Error: template std.range.chain does not match any function
> template declaration. Candidates are:
> std/range.d(2018): std.range.chain(Ranges...)(Ranges rs) if
> (Ranges.length > 0 && allSatisfy!(isInputRange, staticMap!(Unqual,
> Ranges)) && !is(CommonType!(staticMap!(ElementType, staticMap!(Unqual,
> Ranges))) == void))
> 
> That was the first, it list candidates. It's relatively new thing, nice
> to have it.
> 
> bug.d(11): Error: template std.range.chain(Ranges...)(Ranges rs) if
> (Ranges.length > 0 && allSatisfy!(isInputRange, staticMap!(Unqual,
> Ranges)) && !is(CommonType!(staticMap!(ElementType, staticMap!(Unqual,
> Ranges))) == void)) cannot deduce template function from argument types
> !()(MapResult!(__lambda2, Foo[]),string)
> 
> The second need not to realist constraints here, they are obvious from
> above, what needed is the last line - template arguments. So just make it:
> 
> bug.d(11): Error: template std.range.chain(Ranges...)(Ranges rs) cannot
> deduce template function from argument types !()(MapResult!(__lambda2,
> Foo[]),string)
> 
> Bugzilla worthy?

Duplicate errors are definitely bugzilla worthy, and duplicated information 
within errors is bugzilla worthy - _especially_ in template error messages. 
We want them to be informative, but let's not make them longer than they need 
to be. It's the error messages that generally scare people away from 
templates.

- Jonathan M Davis
Top | Discussion index | About this forum | D home