patch via mailing list: Use getLocation() in too few/many arguments diagnostic

[Hubert Tong via cfe-commits]

I think this looks okay. I think Richard or Aaron might be able to provide
a more informed opinion.

-- HT

On Fri, Feb 7, 2020 at 10:06 AM John Marshall via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> Ping. I am a newcomer to Clang so don't know who might be appropriate
> reviewers to CC, so I've CCed a couple of general people from
> clang/CODE_OWNERS.TXT who may be able to forward as appropriate.
>
> Thanks,
>
>     John
>
>
> On 20 Jan 2020, at 16:09, John Marshall wrote:
> >
> > This small patch improves the diagnostics when calling a function with
> the wrong number of arguments. For example, given
> >
> >       int
> >       foo(int a, int b);
> >
> >       int bar() { return foo(1); }
> >
> > The existing compiler shows the error message and a note for "foo
> declared here" showing only the "int" line. Ideally it would show the line
> containing the word "foo" instead, which it does after this patch. Also if
> the function declaration starts with some incidental macro, a trace of
> macro expansion is shown which is likely irrelevant. See for example
> https://github.com/samtools/htslib/issues/1013 and PR#23564.
> >
> > I have not contributed to LLVM before and I am unfamiliar with the
> difference between getBeginLoc() and getLocation() and the implications of
> using each. However this patch does fix PR#23564 and perhaps this fix would
> be mostly a no-brainer for those who are familiar with the code and these
> two location functions in particular?
>
> commit e8e73a04dd4274441541cc5fdc553cc4b26c00c3
> Author: John Marshall <John.W.Marshall at glasgow.ac.uk>
> Date:   Mon Jan 20 14:58:14 2020 +0000
>
>     Use getLocation() in too few/many arguments diagnostic
>
>     Use the more accurate location when emitting the location of the
>     function being called's prototype in diagnostics emitted when calling
>     a function with an incorrect number of arguments.
>
>     In particular, avoids showing a trace of irrelevant macro expansions
>     for "MY_EXPORT static int AwesomeFunction(int, int);". Fixes PR#23564.
>
> diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
> index ffe72c98356..b9d7024f083 100644
> --- a/clang/lib/Sema/SemaExpr.cpp
> +++ b/clang/lib/Sema/SemaExpr.cpp
> @@ -5194,7 +5194,7 @@ Sema::ConvertArgumentsForCall(CallExpr *Call, Expr
> *Fn,
>
>        // Emit the location of the prototype.
>        if (!TC && FDecl && !FDecl->getBuiltinID() && !IsExecConfig)
> -        Diag(FDecl->getBeginLoc(), diag::note_callee_decl) << FDecl;
> +        Diag(FDecl->getLocation(), diag::note_callee_decl) << FDecl;
>
>        return true;
>      }
> @@ -5239,7 +5239,7 @@ Sema::ConvertArgumentsForCall(CallExpr *Call, Expr
> *Fn,
>
>        // Emit the location of the prototype.
>        if (!TC && FDecl && !FDecl->getBuiltinID() && !IsExecConfig)
> -        Diag(FDecl->getBeginLoc(), diag::note_callee_decl) << FDecl;
> +        Diag(FDecl->getLocation(), diag::note_callee_decl) << FDecl;
>
>        // This deletes the extra arguments.
>        Call->shrinkNumArgs(NumParams);
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200207/85a68d1e/attachment.html>