Thank you for your detailed explanation! It would seem perfectly reasonable to define the behavior in this case, at least (and I suppose not only) to me. Attila
2016-12-20 14:50 GMT+01:00 Aaron Ballman <aaron.ball...@gmail.com>: > 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
