[PATCH] D23921: Remove va_start diagnostic false positive with enumerations

[Attila Török via cfe-commits]

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.ballman at gmail.com>:

> On Tue, Dec 20, 2016 at 7:58 AM, Attila Török <torokati44 at 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.ballman at gmail.com>:
> >>
> >> On Fri, Dec 16, 2016 at 7:00 AM, Attila Török via Phabricator
> >> <reviews at 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
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20161220/f45caa80/attachment.html>