Merge pull request #8638 from apple/revert-wformat-scoped-enum

ravikandhadai · web-flow · commit a8b8b0ee537c · 2024-05-01T08:23:54.000-07:00
Revert -Wformat for scoped enums
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
@@ -497,8 +497,10 @@ Improvements to Clang's diagnostics
 - ``-Wformat`` cast fix-its will now suggest ``static_cast`` instead of C-style casts
   for C++ code.
 - ``-Wformat`` will no longer suggest a no-op fix-it for fixing scoped enum format
-  warnings. Instead, it will suggest casting the enum object based on its
-  underlying type.
+  warnings. Instead, it will suggest casting the enum object to the type specified
+  in the format string.
+- Clang contexpr evaluator now displays notes as well as an error when a constructor
+  of a base class is not called in the constructor of its derived class.
 
 - ``-Wzero-as-null-pointer-constant`` diagnostic is no longer emitted when using ``__null``
   (or, more commonly, ``NULL`` when the platform defines it as ``__null``) to be more consistent
@@ -670,11 +672,6 @@ Bug Fixes in This Version
   (`#50244 <https://github.com/llvm/llvm-project/issues/50244>`_).
 - Apply ``-fmacro-prefix-map`` to anonymous tags in template arguments
   (`#63219 <https://github.com/llvm/llvm-project/issues/63219>`_).
-- Clang now properly diagnoses format string mismatches involving scoped
-  enumeration types. A scoped enumeration type is not promoted to an integer
-  type by the default argument promotions, and thus this is UB. Clang's
-  behavior now matches GCC's behavior in C++.
-  (`#38717 <https://github.com/llvm/llvm-project/issues/38717>`_).
 - Fixed a failing assertion when implicitly defining a function within a GNU
   statement expression that appears outside of a function block scope. The
   assertion was benign outside of asserts builds and would only fire in C.
diff --git a/clang/lib/AST/FormatString.cpp b/clang/lib/AST/FormatString.cpp
@@ -351,12 +351,10 @@ ArgType::matchesType(ASTContext &C, QualType argTy) const {
     case AnyCharTy: {
       if (const auto *ETy = argTy->getAs<EnumType>()) {
         // If the enum is incomplete we know nothing about the underlying type.
-        // Assume that it's 'int'. Do not use the underlying type for a scoped
-        // enumeration.
+        // Assume that it's 'int'.
         if (!ETy->getDecl()->isComplete())
           return NoMatch;
-        if (ETy->isUnscopedEnumerationType())
-          argTy = ETy->getDecl()->getIntegerType();
+        argTy = ETy->getDecl()->getIntegerType();
       }
 
       if (const auto *BT = argTy->getAs<BuiltinType>()) {
@@ -393,11 +391,10 @@ ArgType::matchesType(ASTContext &C, QualType argTy) const {
     case SpecificTy: {
       if (const EnumType *ETy = argTy->getAs<EnumType>()) {
         // If the enum is incomplete we know nothing about the underlying type.
-        // Assume that it's 'int'. Do not use the underlying type for a scoped
-        // enumeration as that needs an exact match.
+        // Assume that it's 'int'.
         if (!ETy->getDecl()->isComplete())
           argTy = C.IntTy;
-        else if (ETy->isUnscopedEnumerationType())
+        else
           argTy = ETy->getDecl()->getIntegerType();
       }
       argTy = C.getCanonicalType(argTy).getUnqualifiedType();
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
@@ -11500,26 +11500,18 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
       ImplicitMatch == ArgType::NoMatchTypeConfusion)
     Match = ImplicitMatch;
   assert(Match != ArgType::MatchPromotion);
-
-  // Look through unscoped enums to their underlying type.
+  // Look through 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 = 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;
-    } else {
-      IsScopedEnum = true;
-    }
+    ExprTy = EnumTy->getDecl()->getIntegerType();
+    IsEnum = true;
   }
 
   // %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() &&
@@ -11555,10 +11547,8 @@ 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 or when they're the
-      // underlying type of a scoped enum (which always needs a cast).
-      if (!IsScopedEnum &&
-          (CastTyName == "NSInteger" || CastTyName == "NSUInteger") &&
+      // (long in ASTContext). Only complain to pedants.
+      if ((CastTyName == "NSInteger" || CastTyName == "NSUInteger") &&
           (AT.isSizeT() || AT.isPtrdiffT()) &&
           AT.matchesType(S.Context, CastTy))
         Match = ArgType::NoMatchPedantic;
@@ -11613,15 +11603,20 @@ 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). We should also do so for scoped enumerations.
+      // if necessary).
       SmallString<16> CastBuf;
       llvm::raw_svector_ostream CastFix(CastBuf);
       CastFix << (S.LangOpts.CPlusPlus ? "static_cast<" : "(");
-      IntendedTy.print(CastFix, S.Context.getPrintingPolicy());
+      if (IsScopedEnum) {
+        CastFix << AT.getRepresentativeType(S.Context).getAsString(
+            S.Context.getPrintingPolicy());
+      } else {
+        IntendedTy.print(CastFix, S.Context.getPrintingPolicy());
+      }
       CastFix << (S.LangOpts.CPlusPlus ? ">" : ")");
 
       SmallVector<FixItHint,4> Hints;
-      if (AT.matchesType(S.Context, IntendedTy) != ArgType::Match ||
+      if ((!AT.matchesType(S.Context, IntendedTy) && !IsScopedEnum) ||
           ShouldNotPrintDirectly)
         Hints.push_back(FixItHint::CreateReplacement(SpecRange, os.str()));
 
@@ -11649,7 +11644,7 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
         Hints.push_back(FixItHint::CreateInsertion(After, ")"));
       }
 
-      if (ShouldNotPrintDirectly && !IsScopedEnum) {
+      if (ShouldNotPrintDirectly) {
         // 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
diff --git a/clang/test/FixIt/format.cpp b/clang/test/FixIt/format.cpp
@@ -12,33 +12,21 @@ 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: 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}:")"
+  // CHECK: "static_cast<short>("
 
   printf("%hu", N::E::One); // expected-warning{{format specifies type 'unsigned short' but the argument has type 'N::E'}}
-  // 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}:")"
+  // CHECK: "static_cast<unsigned short>("
 
   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}:")"
@@ -70,8 +58,4 @@ 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}:")"
 }
diff --git a/clang/test/SemaCXX/format-strings.cpp b/clang/test/SemaCXX/format-strings.cpp
@@ -213,28 +213,4 @@ void f() {
 
 
 }
-
-namespace ScopedEnumerations {
-enum class Scoped1 { One };
-enum class Scoped2 : unsigned short { Two };
-
-void f(Scoped1 S1, Scoped2 S2) {
-  printf("%hhd", S1); // expected-warning {{format specifies type 'char' but the argument has type 'Scoped1'}}
-  printf("%hd", S1);  // expected-warning {{format specifies type 'short' but the argument has type 'Scoped1'}}
-  printf("%d", S1);   // expected-warning {{format specifies type 'int' but the argument has type 'Scoped1'}}
-
-  printf("%hhd", S2); // expected-warning {{format specifies type 'char' but the argument has type 'Scoped2'}}
-  printf("%hd", S2);  // expected-warning {{format specifies type 'short' but the argument has type 'Scoped2'}}
-  printf("%d", S2);   // expected-warning {{format specifies type 'int' but the argument has type 'Scoped2'}}
-
-  scanf("%hhd", &S1); // expected-warning {{format specifies type 'char *' but the argument has type 'Scoped1 *'}}
-  scanf("%hd", &S1);  // expected-warning {{format specifies type 'short *' but the argument has type 'Scoped1 *'}}
-  scanf("%d", &S1);  // expected-warning {{format specifies type 'int *' but the argument has type 'Scoped1 *'}}
-
-  scanf("%hhd", &S2); // expected-warning {{format specifies type 'char *' but the argument has type 'Scoped2 *'}}
-  scanf("%hd", &S2);  // expected-warning {{format specifies type 'short *' but the argument has type 'Scoped2 *'}}
-  scanf("%d", &S2);  // expected-warning {{format specifies type 'int *' but the argument has type 'Scoped2 *'}}
-}
-}
-
 #endif
