On Mon, 9 Sep 2013, Jakub Jelinek wrote: > On Mon, Sep 09, 2013 at 11:37:31AM +0200, Richard Biener wrote: > > > > > > this patchlet fixes the column # of the unused parameter warnings > > > > > > emitted > > > > > > by > > > > > > do_warn_unused_parameter by explicitly passing DECL_SOURCE_LOCATION > > > > > > (decl) > > > > > > instead of wrongly relying on '+', which in this case ends up > > > > > > meaning the > > > > > > location of the function declaration. Tested x86_64-linux. > > > > > I would have expected %q+D to use the location of the corresponding > > > > > decl, not some random other location. So, isn't the bug in the > > > > > C++ frontend diagnostic machinery? > > > > Well, first notice that the patch fixes the issue *both* for the C and > > > > C++ > > > > front-ends, that's why I added the testcase to c-c++-common. This isn't > > > > a C++ > > > > issue. Then notice that we do already have tens of cases where we use > > > > DECL_SOURCE_LOCATION + %qD, when we want to be precise about the > > > > location. The > > > > diagnostic machinery has this mechanism using + which uses location_of, > > > > which > > > > is often useful for expressions, but which very often we don't use for > > > > decls. > > > > In fact, some people, like Manuel, see the audit trail of the bug, find > > > > the > > > > mechanism quite confusing. Is there something specific you want me to > > > > check? > > > > > > How is '+' in %q+D defined? I failed to find documentation of the > > > diagnostic formats (but only searched for like two minutes). > > > > That said, grepping for %q+D reveals quite some uses and it looks like > > all of them expect the location being used to be that of the decl passed > > to the diagnostic call, not some random other location. > > The C++ FE locus handling is in very bad shape (C FE is much better, Aldy > and others have spent quite some time fixing all the issues). I guess this > is just one of the many issues.
Well, in this case the patch should IMHO be a no-op. - warning (OPT_Wunused_parameter, "unused parameter %q+D", decl); + warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wunused_parameter, + "unused parameter %qD", decl); no? Unless I misunderstand what %q+D should do. Richard.
