[PATCH] D67775: [Sema] Split out -Wformat-type-confusion from -Wformat-pedantic
Aaron Ballman via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 3 12:20:12 PDT 2019
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
More information about the cfe-commits
mailing list