Better error messages - from reddit

[Johannes Pfau]

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