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

[John Marshall via cfe-commits]

This patch has been languishing for 10 days, and it has been pointed out on cfe-dev that it is important to directly CC appropriate reviewers. As there's no-one listed in clang/CODE_OWNERS.TXT for diagnostics, I've now CCed "all other parts" Richard.

On 20 Jan 2020, at 16:09, John Marshall via cfe-commits <cfe-commits at lists.llvm.org> 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?
> 
> Thanks,
> 
>    John

commit 0fcec5ffe9bc048907f623635b3acf8731ac9ffd
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 2380a6b8d67..20c33c40c64 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -5193,7 +5193,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;
     }
@@ -5238,7 +5238,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);