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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits