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@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.marsh...@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@lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits