aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land.
LGTM (aside from some small nits that you can fix when landing). ================ 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) & ---------------- 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. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130301/new/ https://reviews.llvm.org/D130301 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
