On Tue, Dec 20, 2016 at 7:58 AM, Attila Török <torokat...@gmail.com> wrote: > I did not see that it was reapplied later, sorry. > > With clang version 3.9.0 (tags/RELEASE_390/final) I get the warning in both > c11 and c++11 mode. > With clang version 4.0.0 (trunk 290146) (llvm/trunk 290118) it's gone in c11 > mode, but still there in c++11. > The piece of code I tested it on: > https://gist.github.com/torokati44/37e6aca2d516cb7c3cb31b7ccf8a519e > > In the part of the code affected by the patch, ED->getPromotionType() is > BuiltinType 'int', and Type is EnumType 'enum E'. > For these two types, Context.typesAreCompatible returns true in c11 mode, > but false in c++11 mode (regardless of which underlying type - or if any - > is specified). > I presume this is the intended behavior. And if so, how could the example > code be modified to make it warning-free in c++, while keeping the parameter > an enum, and not making it a simple int?
Yes, that behavior is intended. The answer to how to modify the code involves the C++ standards committee, though. [cstdarg.syn]p1 says, in part, The parameter parmN is the identifier of the rightmost parameter in the variable parameter list of the function definition (the one just before the ...). If the parameter parmN is of a reference type, or of a type that is not compatible with the type that results when passing an argument for which there is no parameter, the behavior is undefined. When typesAreCompatible() returns false, that means you are triggering UB with your code. The reason the C++ behavior differs from the C behavior when calling typesAreCompaible() is because type compatibility (as a language construct) is a C notion; in C++ this maps to "are the types the same", which is not true for an enum type and an int type (even with an explicit underlying type). However, there are questions as to whether this should be UB or not that I've raised with WG21 (I don't have a Core defect report for it yet though). For right now, the safest answer is: change the parameter to be an int, because that is definitely not UB. There may be a more satisfactory answer in the future. ~Aaron > > Thank you, > Attila > > 2016-12-19 18:36 GMT+01:00 Aaron Ballman <aaron.ball...@gmail.com>: >> >> On Fri, Dec 16, 2016 at 7:00 AM, Attila Török via Phabricator >> <revi...@reviews.llvm.org> wrote: >> > torokati44 added a comment. >> > >> > I see this has been reverted in r281612, but I can no longer access the >> > build log serving as a reason linked in the message: >> > https://www.mail-archive.com/cfe-commits@lists.llvm.org/msg35386.html >> > We have a few false-positive warnings that (I think) would be silenced >> > by this change. I'm just wondering if something like this, if not this, >> > could be included anyway? Not critical of course, it just would be nice. >> >> This patch was reapplied in r281632: >> >> >> http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20160912/170772.html >> >> Do you still have false positives even with that applied? >> >> Thanks! >> >> ~Aaron > > _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
