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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to