[PATCH] D63856: [ObjC] Add a -Wtautological-compare warning for BOOL

Erik Pilkington via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 8 16:46:07 PDT 2019


erik.pilkington added a comment.

In D63856#1561160 <https://reviews.llvm.org/D63856#1561160>, @rjmccall wrote:

> In D63856#1561132 <https://reviews.llvm.org/D63856#1561132>, @erik.pilkington wrote:
>
> > In D63856#1561112 <https://reviews.llvm.org/D63856#1561112>, @rjmccall wrote:
> >
> > > In D63856#1560213 <https://reviews.llvm.org/D63856#1560213>, @erik.pilkington wrote:
> > >
> > > > In D63856#1560180 <https://reviews.llvm.org/D63856#1560180>, @rjmccall wrote:
> > > >
> > > > > This only applies to relational operators, right?  I'm a little uncomfortable with calling this "tautological" since it's not like it's *undefined behavior* to have `(BOOL) 2`, it's just *unwise*.  But as long as we aren't warning about reasonable idioms that are intended to handle unfortunate situations — like other code that might have left a non-`{0,1}` value in the `BOOL` — I think this is fine.
> > > >
> > > >
> > > > I think the party line is that it is undefined behaviour (in some sense), since UBSan will happily crash if you try to load a non-boolean value from a BOOL.
> > >
> > >
> > > What?  Since when?
> >
> >
> > https://reviews.llvm.org/D27607
>
>
> Interesting; I'm not sure I find that convincing.  Probably the least convincing part is where it links to its own behavioral documentation as justification for doing what it does.  But okay, I guess we do this.


Hmm, I guess you could say that they were being a little... tautological.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63856/new/

https://reviews.llvm.org/D63856





More information about the cfe-commits mailing list