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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thank you for working on this! FWIW, precommit Ci found a relevant test failure that should also be addressed.



================
Comment at: clang/lib/Sema/SemaExpr.cpp:1062-1073
+  // 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:
----------------
Oh fun, C and C++ solved this issue differently. In C++, there's an implicit conversion so the nullptr_t is converted to void * on the call, and in C there's an allowance for va_arg to handle nullptr_t as a void *.

And without further changes, C++ picks that rule up automatically once it rebases onto C23: http://eel.is/c++draft/support#cstdarg.syn-1


================
Comment at: clang/lib/Sema/SemaExpr.cpp:17328
+      PromoteType = Context.VoidPtrTy;
+    if (TInfo->getType()->isArrayType())
+      PromoteType = Context.getArrayDecayedType(TInfo->getType());
----------------
MitalAshok wrote:
> 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*`.
`_Complex double` and `double[2]` are not compatible types in C.

C2x 6.2.7p1: Two types are compatible types if they are the same. Additional rules for determining whether two types are compatible are described in 6.7.2 for type specifiers, in 6.7.3 for type qualifiers, and in 6.7.6 for declarators. ... (The rest is talking about structures, enumerations, and unions).

`_Complex` is a type specifier, so....

C2x 6.7.2p7: Each of the comma-separated multisets designates the same type, except that for bit-fields, it is implementation-defined whether the specifier int designates the same type as signed int or the same type as unsigned int.

(The multisets it describes do not include any array types.)


================
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
----------------
MitalAshok wrote:
> 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
I don't think this should be diagnosed pedantically; the code is correct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156054