aaron.ballman added a comment.

In D67775#1691999 <https://reviews.llvm.org/D67775#1691999>, @erik.pilkington 
wrote:

> Ping!


Sorry for the delayed review -- thank you for working on clearing this up!



================
Comment at: clang/include/clang/AST/FormatString.h:259
+    NoMatch = 0,
+    /// The conversion specifier and the argument type are compatible.
+    Match = 1,
----------------
Can you add: `For instance, "%d" and _Bool.`


================
Comment at: clang/lib/Sema/SemaChecking.cpp:8165
+        if (ImplicitMatch == ArgType::NoMatchTypeConfusion)
+          Match = ArgType::NoMatchTypeConfusion;
       }
----------------
How about:
```
if (ImplicitMatch == ArgType::NoMatchPedantic ||
    ImplicitMatch == ArgType::NoMatchTypeConfusion)
  Match = ImplicitMatch;
```


================
Comment at: clang/test/Sema/format-strings-pedantic.c:1
-// RUN: %clang_cc1 -fsyntax-only -verify -Wformat -Wformat-pedantic -isystem 
%S/Inputs %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wformat -Wformat-type-confusion %s
 
----------------
Are we losing test coverage for `-Wformat-pedantic`, or do we have other tests 
covering that elsewhere? I would have expected this test file's contents to 
exercise pedantic cases.


================
Comment at: clang/test/Sema/format-type-confusion.c:13
+         b, // expected-warning {{format specifies type 'unsigned short' but 
the argument has type '_Bool'}}
+         b, b, b, b, b);
+
----------------
Just double-checking, but the reason we don't diagnose the `%c` here is because 
of `-Wno-format`?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67775/new/

https://reviews.llvm.org/D67775



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to