Page MenuHomePhabricator

Improve error reporting for SFINAE
Needs ReviewPublic

Authored by pfultz2 on Mar 12 2015, 4:21 PM.

Details

Summary

This is to help improve error reporting with SFINAE as part of the bug report here:

http://llvm.org/bugs/show_bug.cgi?id=13309

So now when this program compiles:

template<class T> auto h(T x)->decltype(x.smurf()){return x.smurf();}
void h() {}
template<class T> auto g(T x)->decltype(h(x)){return h(x);}
template<class T> auto f(T x)->decltype(g(x)){return g(x);}
int main(){
  f(3);
}

Clang will report back this error with the true diagnostics:

smurf.cpp:5:3: error: no matching function for call to 'f'
  f(3);
  ^
smurf.cpp:1:42: note: candidate template ignored: substitution failure [with T = int]: member reference base type 'int' is not a structure or union
template<class T> auto h(T x)->decltype(x.smurf()){return x.smurf();}
                                         ^

Now this only does this when there is single overloads involved. If we change
the example to add an extra overload for h, like this:

template<class T> auto h(T x)->decltype(x.smurf()){return x.smurf();}
void h() {}
template<class T> auto g(T x)->decltype(h(x)){return h(x);}
template<class T> auto f(T x)->decltype(g(x)){return g(x);}
int main(){
  f(3);
}

Then we get an error like this instead:

smurf.cpp:6:3: error: no matching function for call to 'f'
  f(3);
  ^
smurf.cpp:3:41: note: candidate template ignored: substitution failure [with T = int]: no matching function for call to 'h'
template<class T> auto g(T x)->decltype(h(x)){return h(x);}
                                        ^

So clang basically stops when there is multiple overloads. If the user needs
to see more detail for this, we could look at adding an option for a full
backtrace in the future.

Either way, I think this is very useful to have, and improves the error
reporting in clang immensely. I would like to get this patch merged into
clang.

Diff Detail

Repository
rL LLVM

Event Timeline

pfultz2 updated this revision to Diff 21885.Mar 12 2015, 4:21 PM
pfultz2 retitled this revision from to Improve error reporting for SFINAE.
pfultz2 updated this object.
pfultz2 edited the test plan for this revision. (Show Details)
pfultz2 added a reviewer: rsmith.
pfultz2 set the repository for this revision to rL LLVM.
pfultz2 added a subscriber: Unknown Object (MLST).
pfultz2 updated this object.Mar 12 2015, 4:23 PM

This needs a testcase added to the test/ directory.

pfultz2 updated this revision to Diff 23290.Apr 6 2015, 1:37 PM

Added tests

rsmith edited edge metadata.Apr 6 2015, 1:45 PM

As noted on PR13309, this is not acceptable as-is -- note that the sample diagnostic:

smurf.cpp:6:3: error: no matching function for call to 'f'
  f(3);
  ^
smurf.cpp:3:41: note: candidate template ignored: substitution failure [with T = int]: no matching function for call to 'h'
template<class T> auto g(T x)->decltype(h(x)){return h(x);}

... gives the user no idea of how we got from a call to f into a call to g. If we produced a stack of 'in instantiation of' notes, this would be fine (note that's exactly what my patch in comment#1/comment#2 of that bug does).

gives the user no idea of how we got from a call to f into a call to g.

For the default case, its better to show the first and last in the trace, as most users don't care about the intermediate steps.

If we produced a stack of 'in instantiation of' notes, this would be fine

Yes, the full back trace can be added in the future(perhaps as a compiler flag). This is the first step towards that direction. There needs to be some refactoring to DeductionFailureInfo so it stores the entire diagnostics(rather than a single diagnostic) to make a full back trace possible.

note that's exactly what my patch in comment#1/comment#2 of that bug does

That seemed to be more of a hack then something to be used in production.

The difference is, the current diagnostics allow the user to figure out what
happened. This change does not -- there's no way to find out which 'f' caused
the problem, and how we got from 'f' to 'g'.

Yes, the user can still figure out how you got from f to g. The user can
either add an extra overload for g or comment out g(and continue so until
the user gets to f). So instead of working forwards from f to g, the
user just works backwards from g to f. However, its less likely a user
would need to do that since 90% of the time users want to get to g and not
to f.

Paul