[PATCH] D156054: [Clang][Sema] DR722 (nullptr and varargs) and missing -Wvarargs diagnostics
aaron.ballman added a comment.
I think this is looking close to good, just had some minor nits and an extra
test case to consider.
Comment at: clang/lib/Sema/SemaExpr.cpp:17255
+ // Check if these are compatible types according to the C rules even in C++
+ // because va_arg is defined in C in terms of C compatibile types
+ static auto IsCompatible = [&](QualType L, QualType R) {
Comment at: clang/lib/Sema/SemaExpr.cpp:17264-17268
+ QualType PromotedType = PromotedExpr.get()->getType().getUnqualifiedType();
+ QualType VAArgType = VAArg->getType().getUnqualifiedType();
+ // If these types are compatible, it was not promoted to an incompatible
type.
+ if (IsCompatible(PromotedType, VAArgType))
+return QualType();
This code is correct, but let's add an explicit test for the subtle edge case
with `_Atomic` types. `getUnqualifiedType()` does not strip atomic qualifiers,
so `int` and `_Atomic int` should remain incompatible.
Comment at: clang/test/Sema/format-pointer.c:1-8
+// 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
Comment at: clang/test/SemaCXX/varargs.cpp:34
enum Unscoped1 { One = 0x7FFF };
- (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'}}
MitalAshok wrote:
> aaron.ballman wrote:
> > MitalAshok wrote:
> > > MitalAshok wrote:
> > > > Unscoped1 is promoted to int when passed to a variadic function.
> > > >
> > > > The underlying type for Unscoped1 is unsigned int, so only Unscoped1
> > > > and unsigned int are compatible, not Unscoped1 and int. An Unscoped1
> > > > passed to a variadic function must be retrieved via va_arg(ap, int).
> > > >
> > > Although I guess the warning is now wrong because even though `void f(int
> > > x, ...) { std::va_list ap; va_start(ap, x); va_arg(ap, Unscoped1); }`
> > > `f(0, Unscoped1{2})` would be UB, `f(0, 2u)` would not be UB.
> > >
> > > The user still should be warned about it, so I could create a new warning
> > > "second argument to 'va_arg' is of promotable enumeration type
> > > 'Unscoped1'; this va_arg may have undefined behavior because arguments of
> > > this enumeration type will be promoted to 'int', not the underlying type
> > > 'unsigned int'", and maybe suggest a fix `Unscoped1{va_arg(ap,
> > > unsigned)}`.
> > >
> > > Or we could ignore it and pretend that int and enums with underlying
> > > types unsigned are compatible for the purposes of va_arg
> > I think we shouldn't warn in this case because of C23 7.16.1.1p2:
> >
> > > 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:
> > > ...
> > > one type is compatible with a signed integer type, the other type is
> > > compatible with the
> > > corresponding unsigned integer type, and the value is representable in
> > > both types;
> >
> > Coupled with C23 6.7.2.2p13:
> >
> > > For all enumerations without a fixed underlying type, each enumerated
> > > type shall be compatible
> > > with char or a signed or an unsigned integer type that is not bool or a
> > > bit-precise integer type. The
> > > choice of type is implementation-defined., but shall be capable of
> > > representing the values of all
> > > the members of the enumeration.
> >
> > WDYT?
> This seems to have changed very recently between [[
> https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3096.pdf | N3096 ]] (April
> C23 draft) and [[ https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3149.zip
> | N3149 ]] (July C23 draft) by [[
> https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3112.pdf | N3112 ]].
>
> For reference, the old wording (also in C17) read:
>
> > one type is a signed integer type, the other type is the corresponding
> > unsigned integer type,
> > and the value is representable in both types;
>
> So with the current rules, yes they would be compatible and this shouldn't
> warn. I've changed it so it checks types with corresponding signedness.
> This seems to have changed very recently between N3096 (April C23 draft) and
> N3149 (July C23 draft) by N3112
Yes, this changed during ballot comment resolution at the June 2023 meeting; it
was US-127 (see https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3148.doc
[PATCH] D156054: [Clang][Sema] DR722 (nullptr and varargs) and missing -Wvarargs diagnostics
MitalAshok added inline comments.
Comment at: clang/test/SemaCXX/varargs.cpp:34
enum Unscoped1 { One = 0x7FFF };
- (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'}}
aaron.ballman wrote:
> MitalAshok wrote:
> > MitalAshok wrote:
> > > Unscoped1 is promoted to int when passed to a variadic function.
> > >
> > > The underlying type for Unscoped1 is unsigned int, so only Unscoped1 and
> > > unsigned int are compatible, not Unscoped1 and int. An Unscoped1 passed
> > > to a variadic function must be retrieved via va_arg(ap, int).
> > >
> > Although I guess the warning is now wrong because even though `void f(int
> > x, ...) { std::va_list ap; va_start(ap, x); va_arg(ap, Unscoped1); }` `f(0,
> > Unscoped1{2})` would be UB, `f(0, 2u)` would not be UB.
> >
> > The user still should be warned about it, so I could create a new warning
> > "second argument to 'va_arg' is of promotable enumeration type 'Unscoped1';
> > this va_arg may have undefined behavior because arguments of this
> > enumeration type will be promoted to 'int', not the underlying type
> > 'unsigned int'", and maybe suggest a fix `Unscoped1{va_arg(ap, unsigned)}`.
> >
> > Or we could ignore it and pretend that int and enums with underlying types
> > unsigned are compatible for the purposes of va_arg
> I think we shouldn't warn in this case because of C23 7.16.1.1p2:
>
> > 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:
> > ...
> > one type is compatible with a signed integer type, the other type is
> > compatible with the
> > corresponding unsigned integer type, and the value is representable in both
> > types;
>
> Coupled with C23 6.7.2.2p13:
>
> > For all enumerations without a fixed underlying type, each enumerated type
> > shall be compatible
> > with char or a signed or an unsigned integer type that is not bool or a
> > bit-precise integer type. The
> > choice of type is implementation-defined., but shall be capable of
> > representing the values of all
> > the members of the enumeration.
>
> WDYT?
This seems to have changed very recently between [[
https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3096.pdf | N3096 ]] (April
C23 draft) and [[ https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3149.zip |
N3149 ]] (July C23 draft) by [[
https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3112.pdf | N3112 ]].
For reference, the old wording (also in C17) read:
> one type is a signed integer type, the other type is the corresponding
> unsigned integer type,
> and the value is representable in both types;
So with the current rules, yes they would be compatible and this shouldn't
warn. I've changed it so it checks types with corresponding signedness.
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
[PATCH] D156054: [Clang][Sema] DR722 (nullptr and varargs) and missing -Wvarargs diagnostics
MitalAshok updated this revision to Diff 553056.
MitalAshok added a comment.
clang-format
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/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 @@
https://cplusplus.github.io/CWG/issues/722.html;>722
CD2
Can nullptr be passed to an ellipsis?
-Unknown
+Clang 18
https://cplusplus.github.io/CWG/issues/726.html;>726
Index: clang/test/SemaCXX/varargs.cpp
===
--- clang/test/SemaCXX/varargs.cpp
+++ clang/test/SemaCXX/varargs.cpp
@@ -30,22 +30,28 @@
// Ensure the correct behavior for promotable type UB checking.
void promotable(int a, ...) {
- enum Unscoped1 { One = 0x7FFF };
+ enum Unscoped1 { U = 0x7FFF };
(void)__builtin_va_arg(ap, Unscoped1); // ok
- enum Unscoped2 { Two = 0x };
+ enum Unscoped2 { I = 0x };
(void)__builtin_va_arg(ap, Unscoped2); // ok
- enum class Scoped { Three };
+ enum Unscoped3 { UL = 0x7FFF };
+ (void)__builtin_va_arg(ap, Unscoped3); // ok
+
+ enum Unscoped4 { L = 0xu };
+ (void)__builtin_va_arg(ap, Unscoped4); // ok
+
+ enum class Scoped { One };
(void)__builtin_va_arg(ap, Scoped); // ok
- enum Fixed : int { Four };
+ enum Fixed : int { Two };
(void)__builtin_va_arg(ap, Fixed); // ok
- enum FixedSmall : char { Five };
+ enum FixedSmall : char { Three };
(void)__builtin_va_arg(ap, FixedSmall); // expected-warning {{second argument to 'va_arg' is of promotable type 'FixedSmall'; this va_arg has undefined behavior because arguments will be promoted to 'int'}}
- enum FixedLarge : long long { Six };
+ enum FixedLarge : long long { Four };
(void)__builtin_va_arg(ap, FixedLarge); // ok
// Ensure that qualifiers are ignored.
@@ -55,6 +61,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); //
[PATCH] D156054: [Clang][Sema] DR722 (nullptr and varargs) and missing -Wvarargs diagnostics
MitalAshok updated this revision to Diff 553055.
MitalAshok added a comment.
enums now considered compatible with unsigned/signed versions of their
underlying type for the purposes of -Wvarargs
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/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 @@
https://cplusplus.github.io/CWG/issues/722.html;>722
CD2
Can nullptr be passed to an ellipsis?
-Unknown
+Clang 18
https://cplusplus.github.io/CWG/issues/726.html;>726
Index: clang/test/SemaCXX/varargs.cpp
===
--- clang/test/SemaCXX/varargs.cpp
+++ clang/test/SemaCXX/varargs.cpp
@@ -30,22 +30,28 @@
// Ensure the correct behavior for promotable type UB checking.
void promotable(int a, ...) {
- enum Unscoped1 { One = 0x7FFF };
+ enum Unscoped1 { U = 0x7FFF };
(void)__builtin_va_arg(ap, Unscoped1); // ok
- enum Unscoped2 { Two = 0x };
+ enum Unscoped2 { I = 0x };
(void)__builtin_va_arg(ap, Unscoped2); // ok
- enum class Scoped { Three };
+ enum Unscoped3 { UL = 0x7FFF };
+ (void)__builtin_va_arg(ap, Unscoped3); // ok
+
+ enum Unscoped4 { L = 0xu };
+ (void)__builtin_va_arg(ap, Unscoped4); // ok
+
+ enum class Scoped { One };
(void)__builtin_va_arg(ap, Scoped); // ok
- enum Fixed : int { Four };
+ enum Fixed : int { Two };
(void)__builtin_va_arg(ap, Fixed); // ok
- enum FixedSmall : char { Five };
+ enum FixedSmall : char { Three };
(void)__builtin_va_arg(ap, FixedSmall); // expected-warning {{second argument to 'va_arg' is of promotable type 'FixedSmall'; this va_arg has undefined behavior because arguments will be promoted to 'int'}}
- enum FixedLarge : long long { Six };
+ enum FixedLarge : long long { Four };
(void)__builtin_va_arg(ap, FixedLarge); // ok
// Ensure that qualifiers are ignored.
@@ -55,6 +61,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
[PATCH] D156054: [Clang][Sema] DR722 (nullptr and varargs) and missing -Wvarargs diagnostics
aaron.ballman added inline comments.
Comment at: clang/test/SemaCXX/varargs.cpp:34
enum Unscoped1 { One = 0x7FFF };
- (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'}}
MitalAshok wrote:
> MitalAshok wrote:
> > Unscoped1 is promoted to int when passed to a variadic function.
> >
> > The underlying type for Unscoped1 is unsigned int, so only Unscoped1 and
> > unsigned int are compatible, not Unscoped1 and int. An Unscoped1 passed to
> > a variadic function must be retrieved via va_arg(ap, int).
> >
> Although I guess the warning is now wrong because even though `void f(int x,
> ...) { std::va_list ap; va_start(ap, x); va_arg(ap, Unscoped1); }` `f(0,
> Unscoped1{2})` would be UB, `f(0, 2u)` would not be UB.
>
> The user still should be warned about it, so I could create a new warning
> "second argument to 'va_arg' is of promotable enumeration type 'Unscoped1';
> this va_arg may have undefined behavior because arguments of this enumeration
> type will be promoted to 'int', not the underlying type 'unsigned int'", and
> maybe suggest a fix `Unscoped1{va_arg(ap, unsigned)}`.
>
> Or we could ignore it and pretend that int and enums with underlying types
> unsigned are compatible for the purposes of va_arg
I think we shouldn't warn in this case because of C23 7.16.1.1p2:
> 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:
> ...
> one type is compatible with a signed integer type, the other type is
> compatible with the
> corresponding unsigned integer type, and the value is representable in both
> types;
Coupled with C23 6.7.2.2p13:
> For all enumerations without a fixed underlying type, each enumerated type
> shall be compatible
> with char or a signed or an unsigned integer type that is not bool or a
> bit-precise integer type. The
> choice of type is implementation-defined., but shall be capable of
> representing the values of all
> the members of the enumeration.
WDYT?
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
[PATCH] D156054: [Clang][Sema] DR722 (nullptr and varargs) and missing -Wvarargs diagnostics
MitalAshok marked an inline comment as not done.
MitalAshok added inline comments.
Comment at: clang/test/SemaCXX/varargs.cpp:34
enum Unscoped1 { One = 0x7FFF };
- (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'}}
MitalAshok wrote:
> Unscoped1 is promoted to int when passed to a variadic function.
>
> The underlying type for Unscoped1 is unsigned int, so only Unscoped1 and
> unsigned int are compatible, not Unscoped1 and int. An Unscoped1 passed to a
> variadic function must be retrieved via va_arg(ap, int).
>
Although I guess the warning is now wrong because even though `void f(int x,
...) { std::va_list ap; va_start(ap, x); va_arg(ap, Unscoped1); }` `f(0,
Unscoped1{2})` would be UB, `f(0, 2u)` would not be UB.
The user still should be warned about it, so I could create a new warning
"second argument to 'va_arg' is of promotable enumeration type 'Unscoped1';
this va_arg may have undefined behavior because arguments of this enumeration
type will be promoted to 'int', not the underlying type 'unsigned int'", and
maybe suggest a fix `Unscoped1{va_arg(ap, unsigned)}`.
Or we could ignore it and pretend that int and enums with underlying types
unsigned are compatible for the purposes of va_arg
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
[PATCH] D156054: [Clang][Sema] DR722 (nullptr and varargs) and missing -Wvarargs diagnostics
MitalAshok marked an inline comment as done.
MitalAshok added inline comments.
Comment at: clang/test/SemaCXX/varargs.cpp:34
enum Unscoped1 { One = 0x7FFF };
- (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'}}
Unscoped1 is promoted to int when passed to a variadic function.
The underlying type for Unscoped1 is unsigned int, so only Unscoped1 and
unsigned int are compatible, not Unscoped1 and int. An Unscoped1 passed to a
variadic function must be retrieved via va_arg(ap, int).
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
[PATCH] D156054: [Clang][Sema] DR722 (nullptr and varargs) and missing -Wvarargs diagnostics
MitalAshok updated this revision to Diff 552357.
MitalAshok added a comment.
clang-format
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 @@
https://cplusplus.github.io/CWG/issues/722.html;>722
CD2
Can nullptr be passed to an ellipsis?
-Unknown
+Clang 18
https://cplusplus.github.io/CWG/issues/726.html;>726
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 = 0x7FFF };
- (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 = 0x };
(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,
[PATCH] D156054: [Clang][Sema] DR722 (nullptr and varargs) and missing -Wvarargs diagnostics
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 @@
https://cplusplus.github.io/CWG/issues/722.html;>722
CD2
Can nullptr be passed to an ellipsis?
-Unknown
+Clang 18
https://cplusplus.github.io/CWG/issues/726.html;>726
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 = 0x7FFF };
- (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 = 0x };
(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) {
+
[PATCH] D156054: [Clang][Sema] DR722 (nullptr and varargs) and missing -Wvarargs diagnostics
aaron.ballman added a comment. Precommit CI still has a related failure, it seems. Comment at: clang/lib/Sema/SemaExpr.cpp:17317-17319 +if (TInfo->getType()->isSpecificBuiltinType(BuiltinType::Float) || +TInfo->getType()->isSpecificBuiltinType(BuiltinType::Half)) PromoteType = Context.DoubleTy; jcranmer-intel wrote: > aaron.ballman wrote: > > Hmmm... the existing code seems wrong to me because it's not paying any > > attention to `FLT_EVAL_METHOD`, but I think it probably should? CC > > @jcranmer-intel @zahiraam for opinions. > > > > Actually, I wonder if the correct approach here is to split > > `Sema::DefaultArgumentPromotion()` up so that we can calculate what the > > default argument promoted type is of the expression independent of > > performing the actual promotion, and call the promotion type calculation > > logic here? > C23 6.5.2.2p6 [draft N3096] says "trailing arguments that have type `float` > are promoted to `double`". `FLT_EVAL_METHOD` shouldn't need to apply here, > since it's largely about reflecting that things like x87 using 80-bit > precision internally, and not actual argument passing. Ah that's good to know, thank you! 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
[PATCH] D156054: [Clang][Sema] DR722 (nullptr and varargs) and missing -Wvarargs diagnostics
jcranmer-intel added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:17317-17319 +if (TInfo->getType()->isSpecificBuiltinType(BuiltinType::Float) || +TInfo->getType()->isSpecificBuiltinType(BuiltinType::Half)) PromoteType = Context.DoubleTy; aaron.ballman wrote: > Hmmm... the existing code seems wrong to me because it's not paying any > attention to `FLT_EVAL_METHOD`, but I think it probably should? CC > @jcranmer-intel @zahiraam for opinions. > > Actually, I wonder if the correct approach here is to split > `Sema::DefaultArgumentPromotion()` up so that we can calculate what the > default argument promoted type is of the expression independent of performing > the actual promotion, and call the promotion type calculation logic here? C23 6.5.2.2p6 [draft N3096] says "trailing arguments that have type `float` are promoted to `double`". `FLT_EVAL_METHOD` shouldn't need to apply here, since it's largely about reflecting that things like x87 using 80-bit precision internally, and not actual argument passing. 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
[PATCH] D156054: [Clang][Sema] DR722 (nullptr and varargs) and missing -Wvarargs diagnostics
MitalAshok updated this revision to Diff 549177.
MitalAshok added a comment.
clang-format
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/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/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 @@
https://cplusplus.github.io/CWG/issues/722.html;>722
CD2
Can nullptr be passed to an ellipsis?
-Unknown
+Clang 18
https://cplusplus.github.io/CWG/issues/726.html;>726
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/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/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);
+
[PATCH] D156054: [Clang][Sema] DR722 (nullptr and varargs) and missing -Wvarargs diagnostics
MitalAshok updated this revision to Diff 549176.
MitalAshok added a comment.
Address feedback; Reuse DefaultArgumentPromotion instead of duplicating its
functionality when building VaArgExpr
There is the added bonus for it working with dependent types somewhat
(`template __builtin_va_arg(ap, T[2]);` will warn in the template
instead of waiting for it to be instantiated).
I can add the check for dependent types back if this is unwanted.
I don't think there are any edge cases where this will give a false warning.
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/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/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 @@
https://cplusplus.github.io/CWG/issues/722.html;>722
CD2
Can nullptr be passed to an ellipsis?
-Unknown
+Clang 18
https://cplusplus.github.io/CWG/issues/726.html;>726
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/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/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
[PATCH] D156054: [Clang][Sema] DR722 (nullptr and varargs) and missing -Wvarargs diagnostics
aaron.ballman added subscribers: jcranmer-intel, zahiraam.
aaron.ballman added inline comments.
Comment at: clang/lib/AST/FormatString.cpp:484-487
+ if (const auto *PT = argTy->getAs()) {
+if (PT->getPointeeType()->isCharType())
+ return Match;
+ }
We can simplify a bit further these days.
Comment at: clang/lib/AST/FormatString.cpp:522-534
+ if (const auto *PT = argTy->getAs()) {
+QualType pointeeTy = PT->getPointeeType();
+if (pointeeTy->isVoidType() || (!Ptr && pointeeTy->isCharType()))
+ return Match;
+return NoMatchPedantic;
+ } else if (argTy->isNullPtrType()) {
+return MatchPromotion;
Fixes to match the usual coding style rules.
Wow, Phab's diff engine struggles with that edit. Basically: no `else` after a
`return`, remove braces on single-line `if`, renamed to `PointeeTy`.
Comment at: clang/lib/Sema/SemaExpr.cpp:1062-1072
+ // 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:
I think this code should live in `Sema::DefaultArgumentPromotion()` because
it's related specifically to default argument promotions:
https://eel.is/c++draft/expr#call-11.sentence-5
Comment at: clang/lib/Sema/SemaExpr.cpp:17317-17319
+if (TInfo->getType()->isSpecificBuiltinType(BuiltinType::Float) ||
+TInfo->getType()->isSpecificBuiltinType(BuiltinType::Half))
PromoteType = Context.DoubleTy;
Hmmm... the existing code seems wrong to me because it's not paying any
attention to `FLT_EVAL_METHOD`, but I think it probably should? CC
@jcranmer-intel @zahiraam for opinions.
Actually, I wonder if the correct approach here is to split
`Sema::DefaultArgumentPromotion()` up so that we can calculate what the default
argument promoted type is of the expression independent of performing the
actual promotion, and call the promotion type calculation logic here?
Comment at: clang/test/Sema/format-pointer.c:39
+ printf("%p", np);
+ scanf("%p", ); // expected-warning {{format specifies type 'void **' but
the argument has type 'nullptr_t *'}}
+ scanf("%p", ); // pedantic-warning {{format specifies type 'void **'
but the argument has type 'nullptr_t **'}}
MitalAshok wrote:
> Should this be a pedantic warning?
>
> If this reads a non-null pointer, the bit pattern of `np` will be messed up
> but `np` will still read as a nullptr.
> If it reads a null pointer, it should be fine.
>
That's a fun scenario, good test case!
C23 7.26.6.2p10: Except in the case of a % specifier, the input item (or, in
the case of a %n directive, the count of input characters) is converted to a
type appropriate to the conversion specifier. If the input item is not a
matching sequence, the execution of the directive fails: this condition is a
matching failure. Unless assignment suppression was indicated by a *, the
result of the conversion is placed in the object pointed to by the first
argument following the format argument that has not already received a
conversion result. If this object does not have an appropriate type, or if the
result of the conversion cannot be represented in the object, the behavior is
undefined.
p12, the p specifier: Matches an implementation-defined set of sequences, which
should be the same as the set of sequences that may be produced by the %p
conversion of the fprintf function. The corresponding argument shall be a
pointer to a pointer of void. The input item is converted to a pointer value in
an implementation-defined manner. If the input item is a value converted
earlier during the same program execution, the pointer that results shall
compare equal to that value; otherwise the behavior of the %p conversion is
undefined.
The corresponding argument is not a pointer to a pointer of void, so it's UB,
not just pedantically so.
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
[PATCH] D156054: [Clang][Sema] DR722 (nullptr and varargs) and missing -Wvarargs diagnostics
MitalAshok added inline comments.
Comment at: clang/test/Sema/format-pointer.c:39
+ printf("%p", np);
+ scanf("%p", ); // expected-warning {{format specifies type 'void **' but
the argument has type 'nullptr_t *'}}
+ scanf("%p", ); // pedantic-warning {{format specifies type 'void **'
but the argument has type 'nullptr_t **'}}
Should this be a pedantic warning?
If this reads a non-null pointer, the bit pattern of `np` will be messed up but
`np` will still read as a nullptr.
If it reads a null pointer, it should be fine.
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
[PATCH] D156054: [Clang][Sema] DR722 (nullptr and varargs) and missing -Wvarargs diagnostics
MitalAshok updated this revision to Diff 547874.
MitalAshok marked an inline comment as done.
MitalAshok added a comment.
Fix scanf("%p", (char**)ptr) for -Wformat-pedantic
Also added more tests for %p and -Wformat
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/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/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 @@
https://cplusplus.github.io/CWG/issues/722.html;>722
CD2
Can nullptr be passed to an ellipsis?
-Unknown
+Clang 18
https://cplusplus.github.io/CWG/issues/726.html;>726
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/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/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
[PATCH] D156054: [Clang][Sema] DR722 (nullptr and varargs) and missing -Wvarargs diagnostics
aaron.ballman added a comment. Precommit CI found a related issue that should be addressed. Comment at: clang/www/cxx_dr_status.html:4376 Can nullptr be passed to an ellipsis? -Unknown +Clang 17 MitalAshok wrote: > 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 I think it's better to use Clang 18, otherwise people will think there's a diagnostic bug with earlier versions. We usually use "Yes" to mean "we've always supported this correctly, as best we can tell". 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
[PATCH] D156054: [Clang][Sema] DR722 (nullptr and varargs) and missing -Wvarargs diagnostics
MitalAshok updated this revision to Diff 547353.
MitalAshok added a comment.
C99 7.15.1.1p2 explicitly allows a char* to be retrieved as a void* with
va_arg, so the -Wformat-pedantic warning was incorrect.
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/FormatString.cpp
clang/lib/Sema/SemaExpr.cpp
clang/test/CXX/drs/dr7xx.cpp
clang/test/CodeGen/xcore-abi.c
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 @@
https://cplusplus.github.io/CWG/issues/722.html;>722
CD2
Can nullptr be passed to an ellipsis?
-Unknown
+Clang 18
https://cplusplus.github.io/CWG/issues/726.html;>726
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, ...);
@@ -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/CodeGen/xcore-abi.c
===
--- clang/test/CodeGen/xcore-abi.c
+++ clang/test/CodeGen/xcore-abi.c
@@ -76,7 +76,8 @@
// CHECK: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[V5]], ptr align 4 [[P]], i32 20, i1 false)
// CHECK: call void @f(ptr noundef [[V5]])
- int* v6 = va_arg (ap, int[4]); // an unusual aggregate type
+ // an unusual aggregate type
+ int* v6 = va_arg (ap, int[4]); // expected-warning{{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 struct C; // expected-note 6{{here}}
[PATCH] D156054: [Clang][Sema] DR722 (nullptr and varargs) and missing -Wvarargs diagnostics
MitalAshok updated this revision to Diff 547311.
MitalAshok added a comment.
No longer warn on printf("%p", nullptr)
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/FormatString.cpp
clang/lib/Sema/SemaExpr.cpp
clang/test/CXX/drs/dr7xx.cpp
clang/test/CodeGen/xcore-abi.c
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 @@
https://cplusplus.github.io/CWG/issues/722.html;>722
CD2
Can nullptr be passed to an ellipsis?
-Unknown
+Clang 18
https://cplusplus.github.io/CWG/issues/726.html;>726
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, ...);
@@ -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/CodeGen/xcore-abi.c
===
--- clang/test/CodeGen/xcore-abi.c
+++ clang/test/CodeGen/xcore-abi.c
@@ -76,7 +76,8 @@
// CHECK: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[V5]], ptr align 4 [[P]], i32 20, i1 false)
// CHECK: call void @f(ptr noundef [[V5]])
- int* v6 = va_arg (ap, int[4]); // an unusual aggregate type
+ // an unusual aggregate type
+ int* v6 = va_arg (ap, int[4]); // expected-warning{{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 struct C; // expected-note 6{{here}}
Index: clang/lib/Sema/SemaExpr.cpp
[PATCH] D156054: [Clang][Sema] DR722 (nullptr and varargs) and missing -Wvarargs diagnostics
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
[PATCH] D156054: [Clang][Sema] DR722 (nullptr and varargs) and missing -Wvarargs diagnostics
MitalAshok updated this revision to Diff 544073.
MitalAshok added a comment.
Target clang 18 instead
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/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 @@
https://cplusplus.github.io/CWG/issues/722.html;>722
CD2
Can nullptr be passed to an ellipsis?
-Unknown
+Clang 18
https://cplusplus.github.io/CWG/issues/726.html;>726
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: 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 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();
+ //
[PATCH] D156054: [Clang][Sema] DR722 (nullptr and varargs) and missing -Wvarargs diagnostics
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 =
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
Can nullptr be passed to an ellipsis?
-Unknown
+Clang 17
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 @@
https://cplusplus.github.io/CWG/issues/722.html;>722
CD2
Can nullptr be passed to an ellipsis?
-Unknown
+Clang 17
https://cplusplus.github.io/CWG/issues/726.html;>726
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
+++
