On 3/14/13 6:15 PM, Walter Bright wrote:
> On 3/14/2013 3:02 PM, H. S. Teoh wrote:
>> Under a concepts system, this would be caught early because the compiler
>> would detect a concept mismatch (Unqual!T != T) when analysing the
>> template code.
>
> How would that be different from simply adding a template constraint?

The difference is without the constraint there's no error. It's all about the default: by default (no constraints) everything is allowed.

Andrei

On 03/14/2013 10:37 PM, Walter Bright wrote:
> On 3/14/2013 1:57 PM, Timon Gehr wrote:
>> The following breaks most of std.range, and most of std.algorithm
>> could likely
>> be broken too, but I am too lazy to investigate.
>
> Please file this with bugzilla.
>

http://d.puremagic.com/issues/show_bug.cgi?id=9724

On Thursday, March 14, 2013 17:54:05 Andrei Alexandrescu wrote:
> On 3/14/13 2:38 PM, Jonathan M Davis wrote:
> > On Thursday, March 14, 2013 13:26:18 Andrei Alexandrescu wrote:
> >> Walter has measured coverage of Phobos unittests a couple of times, it's very high. But I agree it would be nice to have it as a target.
> > 
> > Though this is exactly a case where 100% unit test coverage doesn't mean much. All that means is that each path has been instantiated and run. It doesn't mean that it's covered enough possible instantiations to properly test the template.
> 
> Concepts won't help there either.

Oh, they may not. My point was simply that while 100% code coverage is great, when you're dealing with templated functions, it's generally nowhere near enough (especially as the number of static ifs goes up), because there could easily be other template arguments which pass the template constraint but won't even compile with the template, let alone work correctly. For instance, while we've been getting better at it, it's generally been a weakness of std.range and std.algorithm that reference-type ranges aren't tested properly and don't work properly with various functions, and yet those same functions generally have good code coverage.

Really, it just means that we need to be thorough in the number of template instantiations tested and not think that 100% code coverage is enough, but I can understand people wanting to find ways to get the compiler to catch some of that for them.

- Jonathan M Davis

On Thursday, March 14, 2013 14:40:11 Walter Bright wrote:
> On 3/14/2013 2:26 PM, bearophile wrote:
> > Walter Bright:
> >> I understand this. But my experience in the matter has been that if the tests cover 100% of the code paths, the incidence of undetected bugs in the code goes very, very low.
> > 
> > This is not so much true for template-heavy code.
> 
> I don't believe that without further evidence.

It's been fairly frequent that std.range and std.algorithm don't work correctly with reference type ranges. The only way to catch that is to test those functions with reference type ranges. 100% code coverage won't get you there. I fully agree that it's a big step in the right direction, but I don't think that it's been all that uncommon for there to be bugs in Phobos where some ranges work and some don't when the buggy function had high code coverage - it just was likely to be testing primarily with arrays and not enough with other range types. We've definitely been reducing the number of those types of bugs, but they were caught because people tried to instantiate them with types that the unit tests didn't test.

- Jonathan M Davis

On Thursday, March 14, 2013 17:58:44 Andrei Alexandrescu wrote:
> > b) unifying template fuzzy testing in Phobos
> > 
> > We have lots of these wheels reinvented across Phobos alone.
> 
> Like those dummy ranges in std.algorithm?

I had improved versions of those that I was working on which were more flexible and thorough. I really should get back to sorting those out and submit them (IIRC, I was blocked by bugs which have now been fixed).

- Jonathan M Davis

On Thursday, March 14, 2013 15:18:56 Andrei Alexandrescu wrote:
> On 3/14/13 3:04 PM, H. S. Teoh wrote:
> > I found (and fixed!) a number of missed combinations in std.algorithm:
> > transient ranges, joiner() not using .save on forward ranges, same bug
> > in transposed(), transposed crashing on jagged range of ranges, etc..
> 
> I think you're right but only in part; there may be a bit of a confusion. There's pure semantic checking such as forgetting to use .save that can't be detected statically. Then there's code that e.g. should work for forward ranges but has only been tested with arrays.

Those tend to be somewhat common, though we've been cutting down on them.

> My question was referring to code that has sheer typos that are mechanically detected, which are present in code that has never ever been instantiated. I do recall we found a few, but I think that illustrates a problem with the process, not the language.

Those tend to be _very_ rare. I wouldn't say that we've _never_ seen one In Phobos, but I sure don't remember one. And that's something that 100% code coverage catches easily. It's the semantic issues where it doesn't.

- Jonathan M Davis

On Thu, Mar 14, 2013 at 06:18:58PM -0400, Andrei Alexandrescu wrote:
> On 3/14/13 6:02 PM, H. S. Teoh wrote:
> >On Thu, Mar 14, 2013 at 05:54:05PM -0400, Andrei Alexandrescu wrote:
> >>On 3/14/13 2:38 PM, Jonathan M Davis wrote:
[...]
> >>>Though this is exactly a case where 100% unit test coverage doesn't mean much.  All that means is that each path has been instantiated and run. It doesn't mean that it's covered enough possible instantiations to properly test the template.
> >>
> >>Concepts won't help there either.
> >[...]
> >
> >It does help. For example, if the code wrongly assumes mutability for a particular template type, then it may work for most test cases (frankly, I find const/immutable unittest coverage in Phobos very poor) but fail when some daring user passes const(T) instead of T to the template. For example, you may have accidentally written the equivalent of:
> >
> >	auto func(T)(T t) {
> >		Unqual!T u = t; //<-- spot the bug
> >		...
> >	}
> >
> >Under a concepts system, this would be caught early because the compiler would detect a concept mismatch (Unqual!T != T) when analysing the template code.
> >
> >Currently, though, if there is no unittest that tries instantiating the template with const(T), the bug goes undetected, because in all *tested* instantiations, Unqual!T == T.
> 
> Template constraints start from the most permissive end of the spectrum: by default there's no verification, and constraints add verification.
> 
> With typeclasses it's the other way around: by default nothing is allowed, so code needs to add permissions explicitly.

I like this way of looking at it. So basically, typeclasses force us to state all assumptions up-front, whereas template constraints leave room for oversight (forget to put isForwardRange!R in the constraints but use .save in the function body).


> I agree that in that regard typeclasses are better than template constraints. Of course there are many other aspects to be considered when comparing the two.
[...]

Yes, and I'm not saying that we should discard template constraints and adopt typeclasses wholesale. I can't say I'm totally sold on typeclasses either, because they do introduce their own tradeoffs. But neither should we dismiss the typeclasses approach so lightly.

In fact, now that I think of it, templates in general suffer from this problem: when you write template X(T) { ... }, pretty much anything goes in the template body, even outright ridiculous things that won't compile for *any* template arguments. But more commonly, the template body may assume things about T that don't apply across all types T. You may assume that T is mutable, or that T has a certain member, etc..

What if templates were modified so that assumptions about T have to be stated up front? We don't necessarily have to introduce typeclasses as a separate thing, but we could have the compiler reject template bodies that try to access internals of T that aren't stated up front. E.g.:

	auto myFunc(T)(T t) {
		return t.fun();	// error: T wasn't stated to have a member named .fun
	}

would produce a compile error, whereas:

	auto myFunc(T)(T t)
		if (hasMember!(T, "fun"))
	{
		return t.fun();	// OK: we stated that we expect T to have a .fun member
	}

would be accepted. The stated assumptions can themselves be templates too, which are expanded by the compiler before checking the template body. (Presumably, hasMember will eventually expand to some compiler intrinsic that tells the compiler what assumptions are being made about T.) So we can still continue using things like isInputRange!T, etc., except that now the compiler will catch unstated assumptions.

IOW, make signature constraints mandatory rather than optional. Arguably, signature constraints *should* be mandatory in properly-written generic code anyway, so this can only add value. And it will not break existing code if said code is properly-written.


T

-- 
The right half of the brain controls the left half of the body. This means that only left-handed people are in their right mind. -- Manoj Srivastava

H. S. Teoh:

> What if templates were modified so that assumptions about T have to be
> stated up front? We don't necessarily have to introduce typeclasses as a
> separate thing, but we could have the compiler reject template bodies
> that try to access internals of T that aren't stated up front. E.g.:
>
> 	auto myFunc(T)(T t) {
> 		return t.fun();	// error: T wasn't stated to have a member named .fun
> 	}
>
> would produce a compile error, whereas:
>
> 	auto myFunc(T)(T t)
> 		if (hasMember!(T, "fun"))
> 	{
> 		return t.fun();	// OK: we stated that we expect T to have a .fun member
> 	}
>
> would be accepted. The stated assumptions can themselves be templates
> too, which are expanded by the compiler before checking the template
> body. (Presumably, hasMember will eventually expand to some compiler
> intrinsic that tells the compiler what assumptions are being made about
> T.) So we can still continue using things like isInputRange!T, etc.,
> except that now the compiler will catch unstated assumptions.

What's the difference between this and Concepts (lite)? :-)

Bye,
bearophile

On Thursday, 14 March 2013 at 20:57:57 UTC, Timon Gehr wrote:
> On 03/14/2013 06:51 PM, Andrei Alexandrescu wrote:
>> On 3/14/13 1:48 PM, H. S. Teoh wrote:
>>> I don't agree. Phobos is a prime example. Does Phobos have unittests?
>>> Yes, and lots of them. Does it still have non-compilable template
>>> instantiations? Yes, because unittests can't cover all possibilities --
>>> there are too many possible combinations of template arguments. There
>>> are bound to be untested combinations which don't work but we're unaware
>>> of.
>>
>> If you found a few, that would be great. I don't think you'll have an
>> easy time.
>>
>> Andrei
>>
>
> Challenge accepted. Clearly the Phobos developers do not instantiate their templates before shipping them. :o)
>
> The following breaks most of std.range, and most of std.algorithm could likely be broken too, but I am too lazy to investigate.

All this breakage is just one and the same bug. These ranges, for some reason, are testing Unqual!Range, and then attempt to store an Unqual!Range, which makes no sense, since the passed type is not Unqual!Range, it's Range.

Probably an old C++ habit, where you can cast away constness by copy. This is not the case in D.

In any case, all of std.range (and some of algorithm) need to be stripped of this.

On 3/14/2013 3:20 PM, Andrei Alexandrescu wrote:
> On 3/14/13 6:15 PM, Walter Bright wrote:
>> On 3/14/2013 3:02 PM, H. S. Teoh wrote:
>>> Under a concepts system, this would be caught early because the compiler
>>> would detect a concept mismatch (Unqual!T != T) when analysing the
>>> template code.
>>
>> How would that be different from simply adding a template constraint?
>
> The difference is without the constraint there's no error. It's all about the
> default: by default (no constraints) everything is allowed.

I meant how would a concept be better than a constraint for this example.