[PATCH] D39122: [Sema] Fixes for enum handling for tautological comparison diagnostics
Nicolas Lesser via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Oct 20 12:27:53 PDT 2017
Rakete1111 added inline comments.
================
Comment at: lib/Sema/SemaChecking.cpp:8610
+ if(!S.getLangOpts().CPlusPlus && OtherT->isEnumeralType()) {
+ OtherT = OtherT->getAs<EnumType>()->getDecl()->getIntegerType();
----------------
Please drop the braces.
================
Comment at: lib/Sema/SemaChecking.cpp:8616
+ // Special-case for C++ for enum with one enumerator with value of 0
+ if (OtherRange.Width == 0)
----------------
You missed a dot at the end.
================
Comment at: lib/Sema/SemaChecking.cpp:8665
+ bool Result;
bool ConstIsLowerBound = (Op == BO_LT || Op == BO_LE) ^ RhsConstant;
----------------
You can define `Result` lower down (and make it `const`).
================
Comment at: lib/Sema/SemaChecking.cpp:8713
+ QualType WrittenType = OtherT;
+ if(!S.getLangOpts().CPlusPlus && OtherT->isEnumeralType()) {
+ OtherT = OtherT->getAs<EnumType>()->getDecl()->getIntegerType();
----------------
Same as before. Also, shouldn't this be a function instead of duplicating the same code two times?
================
Comment at: test/Sema/outof-range-enum-constant-compare.c:1
+// RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -fsyntax-only -DUNSIGNED -verify %s
+// RUN: %clang_cc1 -triple=x86_64-pc-win32 -fsyntax-only -DSIGNED -verify %s
----------------
I don't know what the convention is, but I would prefer to use platform independent tests wherever possible. I couldn't find a flag to change the underlying type of an enum, so I'm not sure if my suggestion is even feasible.
Repository:
rL LLVM
https://reviews.llvm.org/D39122
More information about the cfe-commits
mailing list