MitalAshok added inline comments.
================ Comment at: clang/test/SemaCXX/varargs.cpp:34 enum Unscoped1 { One = 0x7FFFFFFF }; - (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