www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - A proposal for better template constraint error reporting.

reply Gor Gyolchanyan <gor.f.gyolchanyan gmail.com> writes:
This idea is still raw, so don't expect me to be able to answer to all
your question. This is just a general direction in which we could move
to improve the error reports, that occur when templates can't be
instantiated.
What if one could add ddoc comments to parts of the constraint, so
that both ddoc generator could output them in a neat way and the
compiler would use them to report errors:

/// Define a state machine transition types.
template transition(ActionType)
	if
	{
		/// Must be a callable type.
		isCallable!ActionType &&
		/// Must return a state.
		is(ReturnType!ActionType : State) &&
		/// Must take exactly two parameters.
		ParameterTypeTuple!(ActionType).length == 2 &&
		/// The first parameter must be an event.
		is(ParameterTypeTuple!(ActionType)[0] : Event) &&
		/// The second parameter must be a state.
		is(ParameterTypeTuple!(ActionType)[1] : State);
	)
{
	alias ParameterTypeTuple!(ActionType)[1] FromState;
	alias ReturnType!ActionType ToState;
	alias ParameterTypeTuple!(ActionType)[0] Event;
}

The ddoc comments, preceding parts of template constraints would be
used to specify why exactly did the template fail to instantiate, for
example:

Error: Cannot instantiate template demo01.transition(ActionType)
because "Must return a state." constraint is not satisfied.
Writing specifications of the template with all different version of
incorrect constraints just to static assert(0) it is too tedious to do
every time.
Oct 26 2011
next sibling parent reply bearophile <bearophileHUGS lycos.com> writes:
Gor Gyolchanyan:

 The ddoc comments, preceding parts of template constraints would be
 used to specify why exactly did the template fail to instantiate, for
 example:

Good. Is it possible to do the same thing with unittests? /// foo unittest { assert(false); } ==> foo unittest failed But maybe this syntax is better: unittest(foo) { assert(false); } Bye, bearophile
Oct 26 2011
parent Jonny Dee <jonnyd gmx.net> writes:
Am 26.10.11 19:16, schrieb Jonathan M Davis:
 On Wednesday, October 26, 2011 10:10 bearophile wrote:
 Gor Gyolchanyan:
 The ddoc comments, preceding parts of template constraints would be
 used to specify why exactly did the template fail to instantiate, for

 example:

/// foo unittest { assert(false); } ==> foo unittest failed But maybe this syntax is better: unittest(foo) { assert(false); }

I'd definitely favor unittest(foo) or unittest("foo") for naming unittest blocks (and that's how it's been proposed previously). And since it would presumably actually affect the name of the function that the unittest block generates, I think that it makes more sense that way rather than making it a comment (it also takes up less vertical space). - Jonathan M Davis

I'd prefer this approach, too. For the following reason. I don't know if you know GoogleTest library [1] for C++. There you define unittests with symbols as names, too. And if you want to run your test suite you can filter the set of test cases to be executed by providing wildcard filters as command line parameters. It's really a nice feature. But OTOH having a more explanatory comment is also a nice thing. Maybe one could consider both approaches? Jonny [1] http://code.google.com/p/googletest/
Oct 26 2011
prev sibling next sibling parent Gor Gyolchanyan <gor.f.gyolchanyan gmail.com> writes:
The first version looks better and the compiler should output the
comment when the unittest fails.
I think all error comments should be marked in a special way to avoid
undesired effects.

On Wed, Oct 26, 2011 at 9:10 PM, bearophile <bearophileHUGS lycos.com> wrot=
e:
 Gor Gyolchanyan:

 The ddoc comments, preceding parts of template constraints would be
 used to specify why exactly did the template fail to instantiate, for
 example:

Good. Is it possible to do the same thing with unittests? /// foo unittest { =A0assert(false); } =3D=3D> foo unittest failed But maybe this syntax is better: unittest(foo) { =A0assert(false); } Bye, bearophile

Oct 26 2011
prev sibling next sibling parent "Jonathan M Davis" <jmdavisProg gmx.com> writes:
On Wednesday, October 26, 2011 10:10 bearophile wrote:
 Gor Gyolchanyan:
 The ddoc comments, preceding parts of template constraints would be
 used to specify why exactly did the template fail to instantiate, for

 example:

/// foo unittest { assert(false); } ==> foo unittest failed But maybe this syntax is better: unittest(foo) { assert(false); }

I'd definitely favor unittest(foo) or unittest("foo") for naming unittest blocks (and that's how it's been proposed previously). And since it would presumably actually affect the name of the function that the unittest block generates, I think that it makes more sense that way rather than making it a comment (it also takes up less vertical space). - Jonathan M Davis
Oct 26 2011
prev sibling next sibling parent Gor Gyolchanyan <gor.f.gyolchanyan gmail.com> writes:
I agree. But how to address the template constraint problem then?

On Wed, Oct 26, 2011 at 9:16 PM, Jonathan M Davis <jmdavisProg gmx.com> wrote:
 On Wednesday, October 26, 2011 10:10 bearophile wrote:
 Gor Gyolchanyan:
 The ddoc comments, preceding parts of template constraints would be
 used to specify why exactly did the template fail to instantiate, for

 example:

/// foo unittest { assert(false); } ==> foo unittest failed But maybe this syntax is better: unittest(foo) { assert(false); }

I'd definitely favor unittest(foo) or unittest("foo") for naming unittest blocks (and that's how it's been proposed previously). And since it would presumably actually affect the name of the function that the unittest block generates, I think that it makes more sense that way rather than making it a comment (it also takes up less vertical space). - Jonathan M Davis

Oct 26 2011
prev sibling next sibling parent "Simen Kjaeraas" <simen.kjaras gmail.com> writes:
On Wed, 26 Oct 2011 17:34:09 +0200, Gor Gyolchanyan  
<gor.f.gyolchanyan gmail.com> wrote:

 This idea is still raw, so don't expect me to be able to answer to all
 your question. This is just a general direction in which we could move
 to improve the error reports, that occur when templates can't be
 instantiated.
 What if one could add ddoc comments to parts of the constraint, so
 that both ddoc generator could output them in a neat way and the
 compiler would use them to report errors:

 /// Define a state machine transition types.
 template transition(ActionType)
 	if
 	{
 		/// Must be a callable type.
 		isCallable!ActionType &&
 		/// Must return a state.
 		is(ReturnType!ActionType : State) &&
 		/// Must take exactly two parameters.
 		ParameterTypeTuple!(ActionType).length == 2 &&
 		/// The first parameter must be an event.
 		is(ParameterTypeTuple!(ActionType)[0] : Event) &&
 		/// The second parameter must be a state.
 		is(ParameterTypeTuple!(ActionType)[1] : State);
 	)
 {
 	alias ParameterTypeTuple!(ActionType)[1] FromState;
 	alias ReturnType!ActionType ToState;
 	alias ParameterTypeTuple!(ActionType)[0] Event;
 }

 The ddoc comments, preceding parts of template constraints would be
 used to specify why exactly did the template fail to instantiate, for
 example:

 Error: Cannot instantiate template demo01.transition(ActionType)
 because "Must return a state." constraint is not satisfied.
 Writing specifications of the template with all different version of
 incorrect constraints just to static assert(0) it is too tedious to do
 every time.

As always with this type of proposal, the case of multiple non-matching templates seems not to have been considered. Given: template foo(T) if ( /// T must be an integer. is(T : long)) {} template foo(T) if ( /// T must be a string of some kind. isSomeString!T) {} template foo(T) if ( /// T must be an input range. isInputRange!T) {} Which template's error messages should be shown? All of them, preceded by the template definition? e.g: bar.d(17): Error: template instance foo!(float) does not match any template declaration bar.d(5): template instance foo(T) if (is(T : long)) error instantiating bar.d(6): T must be an integer. bar.d(7): template instance foo(T) if (isSomeString!T) error instantiating bar.d(8): T must be a string of some kind. bar.d(79): template instance foo(T) if (isInputRange!T) error instantiating bar.d(10): T must be an input range. I was gonna say this would bring problems with more complex constraints, but after a brief look through Phobos, I have seen that most (~90%) of constraints seem to be a single condition, with 2 making up the majority of the remaining, and only one place have I found a constraint with 3 conditions. The reason for this is that it's often factored out to a separate template or function, which helps some, but would perhaps reduce the helpfulness of the error messages here proposed. -- Simen
Oct 26 2011
prev sibling next sibling parent "Jonathan M Davis" <jmdavisProg gmx.com> writes:
On Wednesday, October 26, 2011 10:26 Gor Gyolchanyan wrote:
 I agree. But how to address the template constraint problem then?

It's a completely separate issue. Being able to add comments which appear in template constraint messages would definitely be valuable (though using functions or eponymous templates with descriptive names - such as isForwardRange - mostly fixes the problem) and may very well be worth adding. But it would be all or nothing, whereas your proposal seems to assume that it's possible to give a message based on which part of the template constraint failed. However, at this point, as far as the compiler is concerned either a template constraint succeeds or fails. It doesn't try and figure out which parts succeeded or failed, and I suspect that for it to do so would definitely complicate matters. Still, printing out the whole template constraint with comments included could be valuable. Of greater concern IMHO is the fact that it's always the first template constraint which is given. You could have 10 different overloads of the same template with you trying to use the 7th one, and the error message prints only the first template constraint, even if it really has nothing to do with your problem. But I really don't know how to fix that. It can't possibly know which overload you were actually targetting. All it knows is that none of the template constraints passed. And printing out _all_ of the template constraints could get very messier (e.g. think of what you'd get with std.conv.to if you printed out _all_ of the template constrainst for toImpl). The way that I've taken to solving the problem is to create one template which covers _all_ of the cases for that template and has an appropriate template constraint, and then have a template that _it_ uses which has all of the appropriate overloads, and that more or less solves the problem, but it would be nice if we could find a better solution for it such that the compiler can give better error messages on its own. Overall, I'd say that your suggestion has merit, but I don't think that it's going to work quite as you envisioned. It would probably also be simpler if it were a single comment for the whole constraint (particularly given that it can't give you only one message from inside the constraint anyway), but I don't know. - Jonathan M Davis
Oct 26 2011
prev sibling next sibling parent "Jonathan M Davis" <jmdavisProg gmx.com> writes:
On Wednesday, October 26, 2011 11:38 Jonny Dee wrote:
 I'd prefer this approach, too. For the following reason. I don't know if
 you know GoogleTest library [1] for C++. There you define unittests with
 symbols as names, too. And if you want to run your test suite you can
 filter the set of test cases to be executed by providing wildcard
 filters as command line parameters. It's really a nice feature. But OTOH
 having a more explanatory comment is also a nice thing. Maybe one could
 consider both approaches?

Having named unittest blocks opens up a number of possibilities for improving how unit tests are run, though obviously other improvements would have to be made to druntime to enable them. The use case that always comes to my mind is how you can tell eclipse to run a specific unit test with JUnit rather than telling it to run all of them (though I think that it may still run all of the prior tests in the file in case that affects the result). For large test suites, that could be quite valuable. So, a number of improvements like that could theoretically be done once we have name unittest blocks. The biggest benefit though IMHO is simply that stack traces for exceptions that escape unittest blocks would clearly indicate _which_ unittest block the exception escaped from. - Jonathan M Davis
Oct 26 2011
prev sibling parent Gor Gyolchanyan <gor.f.gyolchanyan gmail.com> writes:
Good points. I think if we add the notion of a predicate to D, things
would change dramatically.
Predicates would become the primary subject of such features, as:
* template constraints
* invariants
* in and out contracts
* unittests
* any other assertions and checks
* overloading
* conditional statements

The predicates would always have a message of failure integrated, so
any time a predicate fails a message is always available to either
throw as an exception, or display as a compile-time error or log in a
file for any reason.
The key point is, that anything, that ends up as a bool is a predicate.

On Wed, Oct 26, 2011 at 10:41 PM, Jonathan M Davis <jmdavisProg gmx.com> wrote:
 On Wednesday, October 26, 2011 10:26 Gor Gyolchanyan wrote:
 I agree. But how to address the template constraint problem then?

It's a completely separate issue. Being able to add comments which appear in template constraint messages would definitely be valuable (though using functions or eponymous templates with descriptive names - such as isForwardRange - mostly fixes the problem) and may very well be worth adding. But it would be all or nothing, whereas your proposal seems to assume that it's possible to give a message based on which part of the template constraint failed. However, at this point, as far as the compiler is concerned either a template constraint succeeds or fails. It doesn't try and figure out which parts succeeded or failed, and I suspect that for it to do so would definitely complicate matters. Still, printing out the whole template constraint with comments included could be valuable. Of greater concern IMHO is the fact that it's always the first template constraint which is given. You could have 10 different overloads of the same template with you trying to use the 7th one, and the error message prints only the first template constraint, even if it really has nothing to do with your problem. But I really don't know how to fix that. It can't possibly know which overload you were actually targetting. All it knows is that none of the template constraints passed. And printing out _all_ of the template constraints could get very messier (e.g. think of what you'd get with std.conv.to if you printed out _all_ of the template constrainst for toImpl). The way that I've taken to solving the problem is to create one template which covers _all_ of the cases for that template and has an appropriate template constraint, and then have a template that _it_ uses which has all of the appropriate overloads, and that more or less solves the problem, but it would be nice if we could find a better solution for it such that the compiler can give better error messages on its own. Overall, I'd say that your suggestion has merit, but I don't think that it's going to work quite as you envisioned. It would probably also be simpler if it were a single comment for the whole constraint (particularly given that it can't give you only one message from inside the constraint anyway), but I don't know. - Jonathan M Davis

Oct 26 2011