[cfe-commits] Patch: PR9751: -Wformat warnings will point to the callsite and give a note if the format string is elsewhere.

[Richard Trieu]

On Thu, Oct 13, 2011 at 3:25 PM, Richard Trieu <rtrieu at google.com> wrote:

> On Thu, Oct 13, 2011 at 2:53 PM, Eli Friedman <eli.friedman at gmail.com>wrote:
>
>> On Thu, Oct 13, 2011 at 2:38 PM, Richard Trieu <rtrieu at google.com> wrote:
>> > http://llvm.org/bugs/show_bug.cgi?id=9751
>> > -Wformat warnings will point to the format string, but no message at the
>> > call site.  This patch will move the warning to always point at the call
>> > site.  If the format string is part of the function call, one warning
>> will
>> > show.  If the format string is defined elsewhere, a warning at the call
>> site
>> > plus a note where the format string is defined.
>> > Also, if a note is present, the fix-it would be moved to the note so
>> they
>> > won't be applied with -fixit.  If the format string was used in multiple
>> > places, fixits may cause problems in other places.
>> > printf("%d %d", 1);
>> > warning: more '%' conversions than data arguments [-Wformat]
>> > printf("%d %d", 1);
>> >            ~^
>> > const char kFormat[] = "%d %d";
>> > printf(kFormat, 1);
>> > test2.c:8:8: warning: more '%' conversions than data arguments
>> [-Wformat]
>> > printf(kFormat, 1);
>> >        ^~~~~~~
>> > test2.c:7:29: note: format string is defined here
>> > const char kFormat[] = "%d %d";
>> >                            ~^
>> > Patch attached an available at http://codereview.appspot.com/5277043/
>>
>> +namespace {
>> +  class StringLiteralFinder
>> +    : public ConstStmtVisitor<StringLiteralFinder> {
>> +    Sema& S;
>> +    const StringLiteral *StringExpr;
>> +    bool StringFound;
>> +  public:
>> [...]
>>
>> Why is StringLiteralFinder necessary?  The caller should know how it
>> found the string literal...
>>
>> That information isn't currently preserved through the diagnostics.  The
> StringLiteralFinder is necessary to determine this after the fact.  I can go
> look again and see if I can propagate that bit of info through the code.
>
>
Turns out, it was easier to to pass around a bool with this info than
I originally thought.  New patch attached.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20111013/a6ebb610/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: format-warnings2.patch
Type: text/x-patch
Size: 36180 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20111013/a6ebb610/attachment.bin>