[cfe-commits] Print additional information when two function pointers are printed in a diagnostic. (issue 5369119)

[richard at metafoo.co.uk]

http://codereview.appspot.com/5369119/diff/2001/include/clang/Basic/DiagnosticSemaKinds.td
File include/clang/Basic/DiagnosticSemaKinds.td (right):

http://codereview.appspot.com/5369119/diff/2001/include/clang/Basic/DiagnosticSemaKinds.td#newcode1018
include/clang/Basic/DiagnosticSemaKinds.td:1018: "%select{| because of
different number of parameters (expected %5 but has %6)"
Having experienced the ambiguity of GHC's 'expected versus inferred'
diagnostics, I think I'd prefer to see just (%5 vs %6), where the
correspondence can be inferred from the ordering.

http://codereview.appspot.com/5369119/diff/2001/include/clang/Basic/DiagnosticSemaKinds.td#newcode1018
include/clang/Basic/DiagnosticSemaKinds.td:1018: "%select{| because of
different number of parameters (expected %5 but has %6)"
Would the wording be improved by using just ": " instead of " because of
"?

http://codereview.appspot.com/5369119/diff/2001/include/clang/Basic/DiagnosticSemaKinds.td#newcode1019
include/clang/Basic/DiagnosticSemaKinds.td:1019: "| type mismatch at
%ordinal5 parameter (expected %6 but has %7)"
Should this be "| because of type mismatch [...]"?

http://codereview.appspot.com/5369119/diff/2001/include/clang/Basic/DiagnosticSemaKinds.td#newcode1019
include/clang/Basic/DiagnosticSemaKinds.td:1019: "| type mismatch at
%ordinal5 parameter (expected %6 but has %7)"
I think 'in %ordinal5 parameter' would read more naturally than 'at
%ordinal5 parameter'.

http://codereview.appspot.com/5369119/diff/2001/include/clang/Basic/DiagnosticSemaKinds.td#newcode1027
include/clang/Basic/DiagnosticSemaKinds.td:1027:
"%select{s|||s||s|s|s}6)}4">;
It seems redundant to include the word 'qualifier' three times in this
diagnostic. Would it be clear enough to say: "different qualifiers (none
vs 'const volatile')"?

http://codereview.appspot.com/5369119/diff/2001/include/clang/Basic/DiagnosticSemaKinds.td#newcode1837
include/clang/Basic/DiagnosticSemaKinds.td:1837: "%select{| has
different number of parameters (expected %3 but has %4)"
expected/has works well here, since it's clear which one is which.

http://codereview.appspot.com/5369119/diff/2001/include/clang/Basic/DiagnosticSemaKinds.td#newcode1840
include/clang/Basic/DiagnosticSemaKinds.td:1840: "| because of different
qualifiers (expected "
Should this be "| has different qualifiers [...]"?

http://codereview.appspot.com/5369119/diff/2001/include/clang/Basic/DiagnosticSemaKinds.td#newcode4050
include/clang/Basic/DiagnosticSemaKinds.td:4050: "%select{| has
different number of parameters (expected %5 but has %6)"
Did you intend for there to be two spaces between the ';' and the 'has'
in these diagnostics? Also, it might be clearer to use a ':' rather than
a ';' for the new message parts, and drop the 'has'. I think it'd be
clear enough to use:
   error: assigning to 'int (*)()' from incompatible type 'char (*)()':
different return types ('int' vs 'char')

http://codereview.appspot.com/5369119/diff/2001/include/clang/Basic/DiagnosticSemaKinds.td#newcode4053
include/clang/Basic/DiagnosticSemaKinds.td:4053: "| because of different
qualifiers (expected "
If you keep the 'has', this should be "| has different qualifiers".

http://codereview.appspot.com/5369119/diff/2001/lib/Sema/SemaOverload.cpp
File lib/Sema/SemaOverload.cpp (right):

http://codereview.appspot.com/5369119/diff/2001/lib/Sema/SemaOverload.cpp#newcode2185
lib/Sema/SemaOverload.cpp:2185: ToType = ToType->getPointeeType();
If you're happy with a tiny bit of scope creep, it'd be nice to also
strip off reference types.

http://codereview.appspot.com/5369119/diff/2001/lib/Sema/SemaOverload.cpp#newcode2233
lib/Sema/SemaOverload.cpp:2233: << FromFunction->getResultType();
error: missing 'return;' before '}'

http://codereview.appspot.com/5369119/diff/2001/lib/Sema/SemaOverload.cpp#newcode2262
lib/Sema/SemaOverload.cpp:2262: return false;
under-indented statement

http://codereview.appspot.com/5369119/