MitalAshok updated this revision to Diff 552356.
MitalAshok added a comment.

Use C's definition of type compatibility even in C++ mode (to not warn about 
enums promoted to their underlying types)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156054

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/FormatString.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CXX/drs/dr7xx.cpp
  clang/test/CodeGen/xcore-abi.c
  clang/test/Sema/format-pointer.c
  clang/test/Sema/format-strings-pedantic.c
  clang/test/Sema/varargs.c
  clang/test/SemaCXX/format-strings-0x-nopedantic.cpp
  clang/test/SemaCXX/varargs.cpp
  clang/www/cxx_dr_status.html

Index: clang/www/cxx_dr_status.html
===================================================================
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -4373,7 +4373,7 @@
     <td><a href="https://cplusplus.github.io/CWG/issues/722.html";>722</a></td>
     <td>CD2</td>
     <td>Can <TT>nullptr</TT> be passed to an ellipsis?</td>
-    <td class="none" align="center">Unknown</td>
+    <td class="unreleased" align="center">Clang 18</td>
   </tr>
   <tr id="726">
     <td><a href="https://cplusplus.github.io/CWG/issues/726.html";>726</a></td>
Index: clang/test/SemaCXX/varargs.cpp
===================================================================
--- clang/test/SemaCXX/varargs.cpp
+++ clang/test/SemaCXX/varargs.cpp
@@ -31,7 +31,7 @@
 // Ensure the correct behavior for promotable type UB checking.
 void promotable(int a, ...) {
   enum Unscoped1 { One = 0x7FFFFFFF };
-  (void)__builtin_va_arg(ap, Unscoped1); // ok
+  (void)__builtin_va_arg(ap, Unscoped1); // expected-warning {{second argument to 'va_arg' is of promotable type 'Unscoped1'; this va_arg has undefined behavior because arguments will be promoted to 'int'}}
 
   enum Unscoped2 { Two = 0xFFFFFFFF };
   (void)__builtin_va_arg(ap, Unscoped2); // ok
@@ -55,6 +55,24 @@
   (void)__builtin_va_arg(ap, unsigned int);
 
   (void)__builtin_va_arg(ap, bool); // expected-warning {{second argument to 'va_arg' is of promotable type 'bool'; this va_arg has undefined behavior because arguments will be promoted to 'int'}}
+
+  (void)__builtin_va_arg(ap, float); // expected-warning {{second argument to 'va_arg' is of promotable type 'float'; this va_arg has undefined behavior because arguments will be promoted to 'double'}}
+  (void)__builtin_va_arg(ap, __fp16); // expected-warning {{second argument to 'va_arg' is of promotable type '__fp16'; this va_arg has undefined behavior because arguments will be promoted to 'double'}}
+
+#if __cplusplus >= 201103L
+  (void)__builtin_va_arg(ap, decltype(nullptr)); // expected-warning {{second argument to 'va_arg' is of promotable type 'decltype(nullptr)' (aka 'std::nullptr_t'); this va_arg has undefined behavior because arguments will be promoted to 'void *'}}
+#endif
+
+  (void)__builtin_va_arg(ap, int[3]); // expected-warning {{second argument to 'va_arg' is of promotable type 'int[3]'; this va_arg has undefined behavior because arguments will be promoted to 'int *'}}
+  (void)__builtin_va_arg(ap, const int[3]); // expected-warning {{second argument to 'va_arg' is of promotable type 'const int[3]'; this va_arg has undefined behavior because arguments will be promoted to 'const int *'}}
+
+  // _BitInts aren't promoted
+  (void)__builtin_va_arg(ap, _BitInt(7));
+  (void)__builtin_va_arg(ap, unsigned _BitInt(7));
+  (void)__builtin_va_arg(ap, _BitInt(32));
+  (void)__builtin_va_arg(ap, unsigned _BitInt(32));
+  (void)__builtin_va_arg(ap, _BitInt(33));
+  (void)__builtin_va_arg(ap, unsigned _BitInt(33));
 }
 
 #if __cplusplus >= 201103L
Index: clang/test/SemaCXX/format-strings-0x-nopedantic.cpp
===================================================================
--- clang/test/SemaCXX/format-strings-0x-nopedantic.cpp
+++ clang/test/SemaCXX/format-strings-0x-nopedantic.cpp
@@ -5,6 +5,7 @@
 extern int printf(const char *restrict, ...);
 }
 
-void f(char *c) {
+void f(char *c, int *q) {
   printf("%p", c);
+  printf("%p", q);
 }
Index: clang/test/Sema/varargs.c
===================================================================
--- clang/test/Sema/varargs.c
+++ clang/test/Sema/varargs.c
@@ -75,6 +75,14 @@
     (void)__builtin_va_arg(args, enum E); // Don't warn here in C
     (void)__builtin_va_arg(args, short); // expected-warning {{second argument to 'va_arg' is of promotable type 'short'}}
     (void)__builtin_va_arg(args, char); // expected-warning {{second argument to 'va_arg' is of promotable type 'char'}}
+
+    // _BitInts aren't promoted
+    (void)__builtin_va_arg(args, _BitInt(7));
+    (void)__builtin_va_arg(args, unsigned _BitInt(7));
+    (void)__builtin_va_arg(args, _BitInt(32)); // OK
+    (void)__builtin_va_arg(args, unsigned _BitInt(32)); // OK
+    (void)__builtin_va_arg(args, _BitInt(33)); // OK
+    (void)__builtin_va_arg(args, unsigned _BitInt(33)); // OK
 }
 
 void f10(int a, ...) {
@@ -121,3 +129,18 @@
   __builtin_va_list va;
   va_start(va, e);
 }
+
+void f15(__builtin_va_list args) {
+  (void)__builtin_va_arg(args, const int);
+  (void)__builtin_va_arg(args, const volatile int);
+  (void)__builtin_va_arg(args, volatile int);
+  (void)__builtin_va_arg(args, int * volatile);
+
+  typedef short vec [[gnu::vector_size(sizeof(short))]];
+  vec v = __builtin_va_arg(args, vec);
+
+#ifdef MS
+  typedef void(__fastcall * attribute_fn)(void);
+  attribute_fn ptr = __builtin_va_arg(args, attribute_fn);
+#endif
+}
Index: clang/test/Sema/format-strings-pedantic.c
===================================================================
--- clang/test/Sema/format-strings-pedantic.c
+++ clang/test/Sema/format-strings-pedantic.c
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wno-format -Wformat-pedantic %s
 // RUN: %clang_cc1 -xobjective-c -fblocks -fsyntax-only -verify -Wno-format -Wformat-pedantic %s
 // RUN: %clang_cc1 -xc++ -fsyntax-only -verify -Wno-format -Wformat-pedantic %s
+// RUN: %clang_cc1 -std=c2x -fsyntax-only -verify -Wno-format -Wformat-pedantic %s
 
 __attribute__((format(printf, 1, 2)))
 int printf(const char *restrict, ...);
@@ -14,7 +15,7 @@
   printf("%p", (id)0); // expected-warning {{format specifies type 'void *' but the argument has type 'id'}}
 #endif
 
-#ifdef __cplusplus
-  printf("%p", nullptr); // expected-warning {{format specifies type 'void *' but the argument has type 'std::nullptr_t'}}
+#if !__is_identifier(nullptr)
+  printf("%p", nullptr);
 #endif
 }
Index: clang/test/Sema/format-pointer.c
===================================================================
--- /dev/null
+++ clang/test/Sema/format-pointer.c
@@ -0,0 +1,53 @@
+// RUN: %clang_cc1 -xc -Wformat %s -verify
+// RUN: %clang_cc1 -xc -Wformat -std=c2x %s -verify
+// RUN: %clang_cc1 -xc++ -Wformat %s -verify
+// RUN: %clang_cc1 -xobjective-c -Wformat -fblocks %s -verify
+// RUN: %clang_cc1 -xobjective-c++ -Wformat -fblocks %s -verify
+// RUN: %clang_cc1 -xc -std=c2x -Wformat %s -pedantic -verify=expected,pedantic
+// RUN: %clang_cc1 -xc++ -Wformat %s -pedantic -verify=expected,pedantic
+// RUN: %clang_cc1 -xobjective-c -Wformat -fblocks -pedantic %s -verify=expected,pedantic
+
+__attribute__((__format__(__printf__, 1, 2)))
+int printf(const char *, ...);
+__attribute__((__format__(__scanf__, 1, 2)))
+int scanf(const char *, ...);
+
+void f(void *vp, const void *cvp, char *cp, signed char *scp, int *ip) {
+  int arr[2];
+
+  printf("%p", cp);
+  printf("%p", cvp);
+  printf("%p", vp);
+  printf("%p", scp);
+  printf("%p", ip);  // pedantic-warning {{format specifies type 'void *' but the argument has type 'int *'}}
+  printf("%p", arr);  // pedantic-warning {{format specifies type 'void *' but the argument has type 'int *'}}
+
+  scanf("%p", &vp);
+  scanf("%p", &cvp);
+  scanf("%p", (void *volatile*)&vp);
+  scanf("%p", (const void *volatile*)&cvp);
+  scanf("%p", &cp);  // pedantic-warning {{format specifies type 'void **' but the argument has type 'char **'}}
+  scanf("%p", &ip);  // pedantic-warning {{format specifies type 'void **' but the argument has type 'int **'}}
+  scanf("%p", &arr);  // expected-warning {{format specifies type 'void **' but the argument has type 'int (*)[2]'}}
+
+#if !__is_identifier(nullptr)
+  typedef __typeof__(nullptr) nullptr_t;
+  nullptr_t np = nullptr;
+  nullptr_t *npp = &np;
+
+  printf("%p", np);
+  scanf("%p", &np);  // expected-warning {{format specifies type 'void **' but the argument has type 'nullptr_t *'}}
+  scanf("%p", &npp);  // pedantic-warning {{format specifies type 'void **' but the argument has type 'nullptr_t **'}}
+#endif
+
+#ifdef __OBJC__
+  id i = 0;
+  void (^b)(void) = ^{};
+
+  printf("%p", i);  // pedantic-warning {{format specifies type 'void *' but the argument has type 'id'}}
+  printf("%p", b);  // pedantic-warning {{format specifies type 'void *' but the argument has type 'void (^)(void)'}}
+  scanf("%p", &i);  // pedantic-warning {{format specifies type 'void **' but the argument has type 'id *'}}
+  scanf("%p", &b);  // pedantic-warning {{format specifies type 'void **' but the argument has type 'void (^*)(void)'}}
+#endif
+
+}
Index: clang/test/CodeGen/xcore-abi.c
===================================================================
--- clang/test/CodeGen/xcore-abi.c
+++ clang/test/CodeGen/xcore-abi.c
@@ -77,6 +77,7 @@
   // CHECK: call void @f(ptr noundef [[V5]])
 
   int* v6 = va_arg (ap, int[4]);  // an unusual aggregate type
+  // expected-warning@-1{{second argument to 'va_arg' is of promotable type 'int[4]'}}
   f(v6);
   // CHECK: [[I:%[a-z0-9]+]] = load ptr, ptr [[AP]]
   // CHECK: [[P:%[a-z0-9]+]] = load ptr, ptr [[I]]
Index: clang/test/CXX/drs/dr7xx.cpp
===================================================================
--- clang/test/CXX/drs/dr7xx.cpp
+++ clang/test/CXX/drs/dr7xx.cpp
@@ -53,6 +53,15 @@
 #endif
 }
 
+namespace dr722 { // dr722: 18
+#if __cplusplus >= 201103L
+  int x = __builtin_printf("%p", nullptr);
+  void f(__builtin_va_list args) {
+    __builtin_va_arg(args, decltype(nullptr)); // expected-warning {{second argument to 'va_arg' is of promotable type 'decltype(nullptr)' (aka 'std::nullptr_t'); this va_arg has undefined behavior because arguments will be promoted to 'void *'}}
+  }
+#endif
+}
+
 namespace dr727 { // dr727: partial
   struct A {
     template<typename T> struct C; // expected-note 6{{here}}
Index: clang/lib/Sema/SemaExpr.cpp
===================================================================
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -923,6 +923,13 @@
     E = Temp.get();
   }
 
+  // C++ [expr.call]p7, per DR722:
+  //   An argument that has (possibly cv-qualified) type std::nullptr_t is
+  //   converted to void* ([conv.ptr]).
+  // (This does not apply to C2x nullptr)
+  if (getLangOpts().CPlusPlus && E->getType()->isNullPtrType())
+    E = ImpCastExprToType(E, Context.VoidPtrTy, CK_NullToPointer).get();
+
   return E;
 }
 
@@ -936,9 +943,9 @@
     //   enumeration, pointer, pointer to member, or class type, the program
     //   is ill-formed.
     //
-    // Since we've already performed array-to-pointer and function-to-pointer
-    // decay, the only such type in C++ is cv void. This also handles
-    // initializer lists as variadic arguments.
+    // Since we've already performed null pointer conversion, array-to-pointer
+    // decay and function-to-pointer decay, the only such type in C++ is cv
+    // void. This also handles initializer lists as variadic arguments.
     if (Ty->isVoidType())
       return VAK_Invalid;
 
@@ -17277,65 +17284,26 @@
         << TInfo->getType()
         << TInfo->getTypeLoc().getSourceRange();
     }
-
-    // Check for va_arg where arguments of the given type will be promoted
-    // (i.e. this va_arg is guaranteed to have undefined behavior).
-    QualType PromoteType;
-    if (Context.isPromotableIntegerType(TInfo->getType())) {
-      PromoteType = Context.getPromotedIntegerType(TInfo->getType());
-      // [cstdarg.syn]p1 defers the C++ behavior to what the C standard says,
-      // and C2x 7.16.1.1p2 says, in part:
-      //   If type is not compatible with the type of the actual next argument
-      //   (as promoted according to the default argument promotions), the
-      //   behavior is undefined, except for the following cases:
-      //     - both types are pointers to qualified or unqualified versions of
-      //       compatible types;
-      //     - one type is a signed integer type, the other type is the
-      //       corresponding unsigned integer type, and the value is
-      //       representable in both types;
-      //     - one type is pointer to qualified or unqualified void and the
-      //       other is a pointer to a qualified or unqualified character type.
-      // Given that type compatibility is the primary requirement (ignoring
-      // qualifications), you would think we could call typesAreCompatible()
-      // directly to test this. However, in C++, that checks for *same type*,
-      // which causes false positives when passing an enumeration type to
-      // va_arg. Instead, get the underlying type of the enumeration and pass
-      // that.
-      QualType UnderlyingType = TInfo->getType();
-      if (const auto *ET = UnderlyingType->getAs<EnumType>())
-        UnderlyingType = ET->getDecl()->getIntegerType();
-      if (Context.typesAreCompatible(PromoteType, UnderlyingType,
-                                     /*CompareUnqualified*/ true))
-        PromoteType = QualType();
-
-      // If the types are still not compatible, we need to test whether the
-      // promoted type and the underlying type are the same except for
-      // signedness. Ask the AST for the correctly corresponding type and see
-      // if that's compatible.
-      if (!PromoteType.isNull() && !UnderlyingType->isBooleanType() &&
-          PromoteType->isUnsignedIntegerType() !=
-              UnderlyingType->isUnsignedIntegerType()) {
-        UnderlyingType =
-            UnderlyingType->isUnsignedIntegerType()
-                ? Context.getCorrespondingSignedType(UnderlyingType)
-                : Context.getCorrespondingUnsignedType(UnderlyingType);
-        if (Context.typesAreCompatible(PromoteType, UnderlyingType,
-                                       /*CompareUnqualified*/ true))
-          PromoteType = QualType();
-      }
-    }
-    if (TInfo->getType()->isSpecificBuiltinType(BuiltinType::Float))
-      PromoteType = Context.DoubleTy;
-    if (!PromoteType.isNull())
-      DiagRuntimeBehavior(TInfo->getTypeLoc().getBeginLoc(), E,
-                  PDiag(diag::warn_second_parameter_to_va_arg_never_compatible)
-                          << TInfo->getType()
-                          << PromoteType
-                          << TInfo->getTypeLoc().getSourceRange());
   }
 
   QualType T = TInfo->getType().getNonLValueExprType(Context);
-  return new (Context) VAArgExpr(BuiltinLoc, E, TInfo, RPLoc, T, IsMS);
+  auto *ResultExpr =
+      new (Context) VAArgExpr(BuiltinLoc, E, TInfo, RPLoc, T, IsMS);
+
+  // Check for va_arg where arguments of the given type will be promoted
+  // (i.e. this va_arg is guaranteed to have undefined behavior).
+  ExprResult PromotedExpr = DefaultArgumentPromotion(ResultExpr);
+  QualType PromoteType = PromotedExpr.get()->getType();
+
+  if (Context.mergeTypes(PromoteType, ResultExpr->getType(), false, true).isNull()) {
+    DiagRuntimeBehavior(
+        TInfo->getTypeLoc().getBeginLoc(), E,
+        PDiag(diag::warn_second_parameter_to_va_arg_never_compatible)
+            << TInfo->getType() << PromoteType
+            << TInfo->getTypeLoc().getSourceRange());
+  }
+
+  return ResultExpr;
 }
 
 ExprResult Sema::ActOnGNUNullExpr(SourceLocation TokenLoc) {
Index: clang/lib/AST/FormatString.cpp
===================================================================
--- clang/lib/AST/FormatString.cpp
+++ clang/lib/AST/FormatString.cpp
@@ -481,20 +481,9 @@
     }
 
     case CStrTy: {
-      const PointerType *PT = argTy->getAs<PointerType>();
-      if (!PT)
-        return NoMatch;
-      QualType pointeeTy = PT->getPointeeType();
-      if (const BuiltinType *BT = pointeeTy->getAs<BuiltinType>())
-        switch (BT->getKind()) {
-          case BuiltinType::Char_U:
-          case BuiltinType::UChar:
-          case BuiltinType::Char_S:
-          case BuiltinType::SChar:
-            return Match;
-          default:
-            break;
-        }
+      if (const auto *PT = argTy->getAs<PointerType>();
+          PT && PT->getPointeeType()->isCharType())
+        return Match;
 
       return NoMatch;
     }
@@ -529,15 +518,21 @@
     }
 
     case CPointerTy:
-      if (argTy->isVoidPointerType()) {
-        return Match;
-      } if (argTy->isPointerType() || argTy->isObjCObjectPointerType() ||
-            argTy->isBlockPointerType() || argTy->isNullPtrType()) {
+      if (const auto *PT = argTy->getAs<PointerType>()) {
+        QualType PointeeTy = PT->getPointeeType();
+        if (PointeeTy->isVoidType() || (!Ptr && PointeeTy->isCharType()))
+          return Match;
         return NoMatchPedantic;
-      } else {
-        return NoMatch;
       }
 
+      if (argTy->isNullPtrType())
+        return MatchPromotion;
+
+      if (argTy->isObjCObjectPointerType() || argTy->isBlockPointerType())
+        return NoMatchPedantic;
+
+      return NoMatch;
+
     case ObjCPointerTy: {
       if (argTy->getAs<ObjCObjectPointerType>() ||
           argTy->getAs<BlockPointerType>())
Index: clang/lib/AST/ASTContext.cpp
===================================================================
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -10266,8 +10266,13 @@
 /// same. See 6.7.[2,3,5] for additional rules.
 bool ASTContext::typesAreCompatible(QualType LHS, QualType RHS,
                                     bool CompareUnqualified) {
-  if (getLangOpts().CPlusPlus)
+  if (getLangOpts().CPlusPlus) {
+    if (CompareUnqualified) {
+      LHS = LHS.getUnqualifiedType();
+      RHS = RHS.getUnqualifiedType();
+    }
     return hasSameType(LHS, RHS);
+  }
 
   return !mergeTypes(LHS, RHS, false, CompareUnqualified).isNull();
 }
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -89,6 +89,8 @@
 
 Resolutions to C++ Defect Reports
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+- Implemented `DR722 <https://wg21.link/CWG722>`_ which promotes ``nullptr`` to ``void*``
+  when passed to a C-style variadic function.
 
 C Language Changes
 ------------------
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to