[PATCH] D130301: [Clang] Fix how we set the NumPositiveBits on an EnumDecl to cover the case of single enumerator with value zero or an empty enum

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.


================
Comment at: compiler-rt/test/ubsan/TestCases/Misc/enum.cpp:27
+  return ((int)e1 != -1) & ((int)e2 != -1) &
+         // CHECK: error: load of value <unknown>, which is not a valid value for type 'enum EBool'
+         ((int)e3 != -1) & ((int)e4 == 1) &
----------------
aaron.ballman wrote:
> shafik wrote:
> > aaron.ballman wrote:
> > > erichkeane wrote:
> > > > What does THIS come from?  What value is unknown?  Shouldn't the -1 be fine?
> > > +1, I'm surprised by the `<unknown>` there, but also... neither `e1` nor `e2` are of type `enum EBool`!
> > So it looks like clang knows that the only valid values for a bool enum is 0 or 1 and it will mask the value accordingly see godbolt for example using `argc` : https://godbolt.org/z/ceb9hPno9
> > 
> > So that would explain why the original test used a `unsigned char*` in order to prompt the diagnostic. 
> > 
> > Looking into the ubsan diagnostic it looks like it treats bool as an unknown value, separate from integer and float. It is not clear to me why it does this but fixing that feels outside the scope of this change since this was part of the original test.
> Thanks for looking into it! I agree that fixing the ubsan diagnostic behavior is out of scope. That said, can you move the CHECK lines so that they come *after* the line of code being checked? I think that's what threw me for a loop regarding the `EBool` confusion I had.
Fixed, I did it like that to keep with the style of the original test but I see how it can be confusing.


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

https://reviews.llvm.org/D130301