[PATCH] D156054: [Clang][Sema] DR722 (nullptr and varargs) and missing -Wvarargs diagnostics

[Mital Ashok via Phabricator via cfe-commits]

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D156054.543283.patch
Type: text/x-patch
Size: 6755 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230723/1afd96e3/attachment-0001.bin>