[cfe-commits] PATCH: Generate more useful messages for out-of-line definition errors

[Kaelyn Uhrain]

On Mon, Jul 25, 2011 at 11:13 AM, Douglas Gregor <dgregor at apple.com> wrote:

>
> On Jul 22, 2011, at 4:19 PM, Kaelyn Uhrain wrote:
>
> Here's a patch to improve the messages for out-of-line definition errors by
> including the function's signature along with its name, and in the candidate
> function notes including the candidates name and signature as namespace
> qualifiers aren't always apparent at the line of code with the declaration.
>
> The patch is also available for review at
> http://codereview.appspot.com/4817047
>
> For example, given a file tmp.cpp containing:
>
> namespace N2 {
>  struct S1;
>
>   namespace N1 {
>    struct S2 {
>      void func(S1*);
>    };
>
>    struct S1 {};
>   }
> }
> void N2::N1::S2::func(S1*) {}
>
> Clang currently reports:
>
> tmp.cpp:12:18: error: out-of-line definition of 'func' does not match any
> declaration in 'N2::N1::S2'
> void N2::N1::S2::func(S1*) {}
>      ~~~~~~~~~~~~^
>
> Sadly, g++ (version 4.4.3) gives better messages:
>
> tmp.cpp:12: error: prototype for 'void N2::N1::S2::func(N2::N1::S1*)' does
> not match any in class 'N2::N1::S2'
> tmp.cpp:6: error: candidate is: void N2::N1::S2::func(N2::S1*)
>
> With this patch, clang yields:
>
> tmp.cpp:12:18: error: out-of-line definition of 'void func(N2::N1::S1 *)'
> does not match any
>       declaration in 'N2::N1::S2'
> void N2::N1::S2::func(S1*) {}
>      ~~~~~~~~~~~~^
> tmp.cpp:6:11: note: member declaration 'void func(N2::S1 *)' nearly matches
>      void func(S1*);
>           ^
>
>
> I'm not convinced we want to go in this direction. Or general principle for
> Clang diagnostics has been "show, don't tell", and we'd much rather point
> out specifically where the mismatch occurs than print out the entire type of
> the function and force the user to walk through the parameters to find the
> mismatch. This approach also allows us to add Fix-Its when the mismatch is
> due to something easily fixed (e.g., missing cv-qualifiers).
>

Clang currently only suggests match candidates if there is a slight mismatch
such as missing cv-qualifers, but all of the underlying (unqualified) types
have to match. My patch just expands that to match cases where the types are
different but have the same name--where there are missing/implicit namespace
qualifers, which currently will give a totally useless and very baffling
message to the user. In the example I gave previously, it says
N2::N1::S2::func(S1*) is unmatched but the user will look in the code and
see func(S1*) declared in struct N2::N1::S2 with no idea why clang wasn't
accepting the message.


> Overload-resolution diagnostics are a good model for what we want here. We
> don't just print out the types of the arguments and the types of the
> candidate functions; instead, we only point out the specific problem that
> caused us to reject each candidate, so users can focus on that
> parameter/argument pair.
>


I actually started to save off which parameters were causing the mismatch so
they could be reported, but I could not come up with a way to mark it
without being too ridiculously verbose (especially if more than one param
was a mismatch). And this is also a case where choosing an appropriate fixit
would be problematic since e.g. in the example from my previous email it is
unclear whether the definition of func(S1*) should be changed to func(::S1*)
or if the first declaration of struct S1 accidentally done at the wrong
namespace scope. I'll take a look at the overload-resolution diagnostic to
see how it handles this kind of mismatch.

Cheers,
Kaelyn
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110725/7dfbcc4c/attachment.html>