[clang] 0b07b06 - [Sema] Use underlying type of scoped enum for -Wformat diagnostics (#67378)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Oct 2 11:33:00 PDT 2023
Author: Shoaib Meenai
Date: 2023-10-02T11:32:54-07:00
New Revision: 0b07b06effe5fdf779b75bb5ac6cf15e477cb0be
URL: https://github.com/llvm/llvm-project/commit/0b07b06effe5fdf779b75bb5ac6cf15e477cb0be
DIFF: https://github.com/llvm/llvm-project/commit/0b07b06effe5fdf779b75bb5ac6cf15e477cb0be.diff
LOG: [Sema] Use underlying type of scoped enum for -Wformat diagnostics (#67378)
Right now, `-Wformat` for a scoped enum will suggest a cast based on the
format specifier being used. This can lead to incorrect results, e.g.
attempting to format a scoped enum with `%s` would suggest casting to
`char *` instead of fixing the specifier. Change the logic to treat the
scoped enum's underlying type as the intended type to be printed, and
suggest format specifier changes and casts based on that.
Added:
clang/test/FixIt/format-darwin-enum-class.cpp
Modified:
clang/docs/ReleaseNotes.rst
clang/lib/Sema/SemaChecking.cpp
clang/test/FixIt/format.cpp
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 0c1023f63aae5d6..6be824771c583be 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -213,6 +213,9 @@ Improvements to Clang's diagnostics
(`#54678: <https://github.com/llvm/llvm-project/issues/54678>`_).
- Clang now prints its 'note' diagnostic in cyan instead of black, to be more compatible
with terminals with dark background colors. This is also more consistent with GCC.
+- The fix-it emitted by ``-Wformat`` for scoped enumerations now take the
+ enumeration's underlying type into account instead of suggesting a type just
+ based on the format string specifier being used.
Bug Fixes in This Version
-------------------------
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 0b9bee414603c6b..3b0cc154c2e46ab 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -11356,12 +11356,15 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
ImplicitMatch == ArgType::NoMatchTypeConfusion)
Match = ImplicitMatch;
assert(Match != ArgType::MatchPromotion);
+
// Look through unscoped enums to their underlying type.
bool IsEnum = false;
bool IsScopedEnum = false;
+ QualType IntendedTy = ExprTy;
if (auto EnumTy = ExprTy->getAs<EnumType>()) {
+ IntendedTy = EnumTy->getDecl()->getIntegerType();
if (EnumTy->isUnscopedEnumerationType()) {
- ExprTy = EnumTy->getDecl()->getIntegerType();
+ ExprTy = IntendedTy;
// This controls whether we're talking about the underlying type or not,
// which we only want to do when it's an unscoped enum.
IsEnum = true;
@@ -11373,7 +11376,6 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
// %C in an Objective-C context prints a unichar, not a wchar_t.
// If the argument is an integer of some kind, believe the %C and suggest
// a cast instead of changing the conversion specifier.
- QualType IntendedTy = ExprTy;
if (isObjCContext() &&
FS.getConversionSpecifier().getKind() == ConversionSpecifier::CArg) {
if (ExprTy->isIntegralOrUnscopedEnumerationType() &&
@@ -11409,8 +11411,10 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
std::tie(CastTy, CastTyName) = shouldNotPrintDirectly(S.Context, IntendedTy, E);
if (!CastTy.isNull()) {
// %zi/%zu and %td/%tu are OK to use for NSInteger/NSUInteger of type int
- // (long in ASTContext). Only complain to pedants.
- if ((CastTyName == "NSInteger" || CastTyName == "NSUInteger") &&
+ // (long in ASTContext). Only complain to pedants or when they're the
+ // underlying type of a scoped enum (which always needs a cast).
+ if (!IsScopedEnum &&
+ (CastTyName == "NSInteger" || CastTyName == "NSUInteger") &&
(AT.isSizeT() || AT.isPtr
diff T()) &&
AT.matchesType(S.Context, CastTy))
Match = ArgType::NoMatchPedantic;
@@ -11465,20 +11469,15 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
// should be printed as 'long' for 64-bit compatibility.)
// Rather than emitting a normal format/argument mismatch, we want to
// add a cast to the recommended type (and correct the format string
- // if necessary).
+ // if necessary). We should also do so for scoped enumerations.
SmallString<16> CastBuf;
llvm::raw_svector_ostream CastFix(CastBuf);
CastFix << (S.LangOpts.CPlusPlus ? "static_cast<" : "(");
- if (IsScopedEnum) {
- CastFix << AT.getRepresentativeType(S.Context).getAsString(
- S.Context.getPrintingPolicy());
- } else {
- IntendedTy.print(CastFix, S.Context.getPrintingPolicy());
- }
+ IntendedTy.print(CastFix, S.Context.getPrintingPolicy());
CastFix << (S.LangOpts.CPlusPlus ? ">" : ")");
SmallVector<FixItHint,4> Hints;
- if ((!AT.matchesType(S.Context, IntendedTy) && !IsScopedEnum) ||
+ if (AT.matchesType(S.Context, IntendedTy) != ArgType::Match ||
ShouldNotPrintDirectly)
Hints.push_back(FixItHint::CreateReplacement(SpecRange, os.str()));
@@ -11506,7 +11505,7 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
Hints.push_back(FixItHint::CreateInsertion(After, ")"));
}
- if (ShouldNotPrintDirectly) {
+ if (ShouldNotPrintDirectly && !IsScopedEnum) {
// The expression has a type that should not be printed directly.
// We extract the name from the typedef because we don't want to show
// the underlying type in the diagnostic.
diff --git a/clang/test/FixIt/format-darwin-enum-class.cpp b/clang/test/FixIt/format-darwin-enum-class.cpp
new file mode 100644
index 000000000000000..5aa1a80d8614c20
--- /dev/null
+++ b/clang/test/FixIt/format-darwin-enum-class.cpp
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -fsyntax-only -verify -Wformat %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -fsyntax-only -fdiagnostics-parseable-fixits -Wformat %s 2>&1 | FileCheck %s
+
+extern "C" int printf(const char * restrict, ...);
+
+#if __LP64__
+typedef long CFIndex;
+typedef long NSInteger;
+typedef unsigned long NSUInteger;
+#else
+typedef int CFIndex;
+typedef int NSInteger;
+typedef unsigned int NSUInteger;
+#endif
+
+enum class CFIndexEnum : CFIndex { One };
+enum class NSIntegerEnum : NSInteger { Two };
+enum class NSUIntegerEnum : NSUInteger { Three };
+
+void f() {
+ printf("%d", CFIndexEnum::One); // expected-warning{{format specifies type 'int' but the argument has type 'CFIndexEnum'}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%ld"
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:16-[[@LINE-2]]:16}:"static_cast<long>("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:32-[[@LINE-3]]:32}:")"
+
+ printf("%d", NSIntegerEnum::Two); // expected-warning{{format specifies type 'int' but the argument has type 'NSIntegerEnum'}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%ld"
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:16-[[@LINE-2]]:16}:"static_cast<long>("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:34-[[@LINE-3]]:34}:")"
+
+ printf("%d", NSUIntegerEnum::Three); // expected-warning{{format specifies type 'int' but the argument has type 'NSUIntegerEnum'}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%lu"
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:16-[[@LINE-2]]:16}:"static_cast<unsigned long>("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:37-[[@LINE-3]]:37}:")"
+}
diff --git a/clang/test/FixIt/format.cpp b/clang/test/FixIt/format.cpp
index 5016ee587ed1c43..4e6573a4f9e54e3 100644
--- a/clang/test/FixIt/format.cpp
+++ b/clang/test/FixIt/format.cpp
@@ -12,21 +12,33 @@ struct S {
N::E Type;
};
+using uint32_t = unsigned;
+enum class FixedE : uint32_t { Two };
+
void a(N::E NEVal, S *SPtr, S &SRef) {
printf("%d", N::E::One); // expected-warning{{format specifies type 'int' but the argument has type 'N::E'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:16-[[@LINE-1]]:16}:"static_cast<int>("
// CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:25-[[@LINE-2]]:25}:")"
printf("%hd", N::E::One); // expected-warning{{format specifies type 'short' but the argument has type 'N::E'}}
- // CHECK: "static_cast<short>("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:14}:"%d"
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:17-[[@LINE-2]]:17}:"static_cast<int>("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:26-[[@LINE-3]]:26}:")"
printf("%hu", N::E::One); // expected-warning{{format specifies type 'unsigned short' but the argument has type 'N::E'}}
- // CHECK: "static_cast<unsigned short>("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:14}:"%d"
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:17-[[@LINE-2]]:17}:"static_cast<int>("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:26-[[@LINE-3]]:26}:")"
LOG("%d", N::E::One); // expected-warning{{format specifies type 'int' but the argument has type 'N::E'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:13-[[@LINE-1]]:13}:"static_cast<int>("
// CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:22-[[@LINE-2]]:22}:")"
+ LOG("%s", N::E::One); // expected-warning{{format specifies type 'char *' but the argument has type 'N::E'}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:8-[[@LINE-1]]:10}:"%d"
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:13-[[@LINE-2]]:13}:"static_cast<int>("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:22-[[@LINE-3]]:22}:")"
+
printf("%d", NEVal); // expected-warning{{format specifies type 'int' but the argument has type 'N::E'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:16-[[@LINE-1]]:16}:"static_cast<int>("
// CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:21-[[@LINE-2]]:21}:")"
@@ -58,4 +70,8 @@ void a(N::E NEVal, S *SPtr, S &SRef) {
SRef.Type);
// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:7}:"static_cast<int>("
// CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:16-[[@LINE-2]]:16}:")"
+
+ printf("%u", FixedE::Two); //expected-warning{{format specifies type 'unsigned int' but the argument has type 'FixedE'}}
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:16-[[@LINE-1]]:16}:"static_cast<uint32_t>("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:27-[[@LINE-2]]:27}:")"
}
More information about the cfe-commits
mailing list