MitalAshok created this revision.
Herald added a project: All.
MitalAshok added a comment.
MitalAshok added a reviewer: aaron.ballman.
MitalAshok published this revision for review.
Herald added subscribers: cfe-commits, wangpc.
Herald added a project: clang.

There is one observable difference by explicitly casting `nullptr_t` to 
`void*`: prvalues of type `nullptr_t` aren't read from (which seemed 
unintentional):

  static void* f(int x, ...) {
      __builtin_va_list args;
      __builtin_va_start(args, x);
      void* arg = __builtin_va_arg(args, void*);
      __builtin_va_end(args);
      return arg;
  }
  
  int main() {
      int x;
      void* x_ptr = &x;
      void* result = f(0, __builtin_bit_cast(decltype(nullptr), x_ptr));
      __builtin_printf("%p\n%p\n%p\n", x_ptr, result, nullptr);
  }

https://godbolt.org/z/Pfv8Wbhxj

An object of type `nullptr_t` is passed to the function, but when it is read 
with `void* arg = __builtin_va_arg(args, void*);`, it is not a null pointer.



================
Comment at: clang/lib/Sema/SemaExpr.cpp:17328
+      PromoteType = Context.VoidPtrTy;
+    if (TInfo->getType()->isArrayType())
+      PromoteType = Context.getArrayDecayedType(TInfo->getType());
----------------
This warns if you call `va_arg(ap, double[2])`. However this might be valid 
since the actual argument only has to be a "compatible type" and I think 
`double _Complex` is compatible with `double[2]`. I think we should warn 
anyways, since array rvalues are tricky to work with, and the user probably 
passed a `double[2]` and should retrieve the `double*`.


================
Comment at: clang/test/Sema/format-strings-pedantic.c:21
+#elif !__is_identifier(nullptr)
+  printf("%p", nullptr); // expected-warning {{format specifies type 'void *' 
but the argument has type 'nullptr_t'}}
 #endif
----------------
In C2x, nullptr passed as an argument and retrieved via `va_arg(ap, void *)` is 
valid (See C2x 7.16.1.1p2) and this is the same as `printf("%p", (void*) 
nullptr)`, but this should still be fine as a "pedantic" warning


================
Comment at: clang/www/cxx_dr_status.html:4376
     <td>Can <TT>nullptr</TT> be passed to an ellipsis?</td>
-    <td class="none" align="center">Unknown</td>
+    <td class="unreleased" align="center">Clang 17</td>
   </tr>
----------------
It may be better to mark this as "Yes", since in old clang versions passing 
nullptr would work by virtue of it having representation of a null void* 
pointer, and it only affected diagnostics


nullptr passed to a variadic function in C++ now converted to void*.
va_arg on array types and half precision floats now diagnosed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156054

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CXX/drs/dr7xx.cpp
  clang/test/Sema/format-strings-pedantic.c
  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 17</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
@@ -55,6 +55,16 @@
   (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 *'}}
 }
 
 #if __cplusplus >= 201103L
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, ...);
@@ -15,6 +16,8 @@
 #endif
 
 #ifdef __cplusplus
-  printf("%p", nullptr); // expected-warning {{format specifies type 'void *' but the argument has type 'std::nullptr_t'}}
+  printf("%p", nullptr);
+#elif !__is_identifier(nullptr)
+  printf("%p", nullptr); // expected-warning {{format specifies type 'void *' but the argument has type 'nullptr_t'}}
 #endif
 }
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: 17
+#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
@@ -936,9 +936,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;
 
@@ -1059,6 +1059,19 @@
 
   E = ExprRes.get();
 
+  // C2x 6.5.2.2p6:
+  //   The integer promotions are performed on each trailing argument, and
+  //   trailing arguments that have type float are promoted to double. These are
+  //   called the default argument promotions. No other conversions are
+  //   performed implicitly.
+
+  // C++ [expr.call]p7, per DR722:
+  //   An argument that has (possibly cv-qualified) type std::nullptr_t is
+  //   converted to void* ([conv.ptr]).
+  if (E->getType()->isNullPtrType() && getLangOpts().CPlusPlus) {
+    E = ImpCastExprToType(E, Context.VoidPtrTy, CK_NullToPointer).get();
+  }
+
   // Diagnostics regarding non-POD argument types are
   // emitted along with format string checking in Sema::CheckFunctionCall().
   if (isValidVarArgType(E->getType()) == VAK_Undefined) {
@@ -17307,8 +17320,13 @@
           PromoteType = QualType();
       }
     }
-    if (TInfo->getType()->isSpecificBuiltinType(BuiltinType::Float))
+    if (TInfo->getType()->isSpecificBuiltinType(BuiltinType::Float) ||
+        TInfo->getType()->isSpecificBuiltinType(BuiltinType::Half))
       PromoteType = Context.DoubleTy;
+    if (TInfo->getType()->isNullPtrType() && getLangOpts().CPlusPlus)
+      PromoteType = Context.VoidPtrTy;
+    if (TInfo->getType()->isArrayType())
+      PromoteType = Context.getArrayDecayedType(TInfo->getType());
     if (!PromoteType.isNull())
       DiagRuntimeBehavior(TInfo->getTypeLoc().getBeginLoc(), E,
                   PDiag(diag::warn_second_parameter_to_va_arg_never_compatible)
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -159,6 +159,8 @@
     // -Wdeprecated-literal-operator diagnoses the extra space.
     string operator "" _i18n(const char*, std::size_t);
     //                ^ an extra space
+- 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