[Sema] Fixes for enum handling for tautological comparison diagnostics
Summary:
As Mattias Eriksson has reported in PR35009, in C, for enums, the underlying type should
be used when checking for the tautological comparison, unlike C++, where the enumerator
values define the value range. So if not in CPlusPlus mode, use the enum underlying type.
Also, i have discovered a problem (a crash) when evaluating tautological-ness of the following comparison:
```
enum A { A_a = 0 };
if (a < 0) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
return 0;
```
This affects both the C and C++, but after the first fix, only C++ code was affected.
That was also fixed, while preserving (i think?) the proper diagnostic output.
And while there, attempt to enhance the test coverage.
Yes, some tests got moved around, sorry about that :)
Fixes PR35009
Reviewers: aaron.ballman, rsmith, rjmccall
Reviewed By: aaron.ballman
Subscribers: Rakete1111, efriedma, materi, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D39122
llvm-svn: 316268
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 79263f1..53e710d 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -8181,8 +8181,12 @@
if (const AtomicType *AT = dyn_cast<AtomicType>(T))
T = AT->getValueType().getTypePtr();
- // For enum types, use the known bit width of the enumerators.
- if (const EnumType *ET = dyn_cast<EnumType>(T)) {
+ if (!C.getLangOpts().CPlusPlus) {
+ // For enum types in C code, use the underlying datatype.
+ if (const EnumType *ET = dyn_cast<EnumType>(T))
+ T = ET->getDecl()->getIntegerType().getDesugaredType(C).getTypePtr();
+ } else if (const EnumType *ET = dyn_cast<EnumType>(T)) {
+ // For enum types in C++, use the known bit width of the enumerators.
EnumDecl *Enum = ET->getDecl();
// In C++11, enums without definitions can have an explicitly specified
// underlying type. Use this type to compute the range.
@@ -8584,8 +8588,10 @@
}
enum class LimitType {
- Max, // e.g. 32767 for short
- Min // e.g. -32768 for short
+ Max = 1U << 0U, // e.g. 32767 for short
+ Min = 1U << 1U, // e.g. -32768 for short
+ Both = Max | Min // When the value is both the Min and the Max limit at the
+ // same time; e.g. in C++, A::a in enum A { a = 0 };
};
/// Checks whether Expr 'Constant' may be the
@@ -8608,6 +8614,10 @@
IntRange OtherRange = IntRange::forValueOfType(S.Context, OtherT);
+ // Special-case for C++ for enum with one enumerator with value of 0.
+ if (OtherRange.Width == 0)
+ return Value == 0 ? LimitType::Both : llvm::Optional<LimitType>();
+
if (llvm::APSInt::isSameValue(
llvm::APSInt::getMaxValue(OtherRange.Width,
OtherT->isUnsignedIntegerType()),
@@ -8620,7 +8630,7 @@
Value))
return LimitType::Min;
- return llvm::Optional<LimitType>();
+ return llvm::None;
}
bool HasEnumType(Expr *E) {
@@ -8655,9 +8665,12 @@
bool ConstIsLowerBound = (Op == BO_LT || Op == BO_LE) ^ RhsConstant;
bool ResultWhenConstEqualsOther = (Op == BO_LE || Op == BO_GE);
- bool ResultWhenConstNeOther =
- ConstIsLowerBound ^ (ValueType == LimitType::Max);
- if (ResultWhenConstEqualsOther != ResultWhenConstNeOther)
+ if (ValueType != LimitType::Both) {
+ bool ResultWhenConstNeOther =
+ ConstIsLowerBound ^ (ValueType == LimitType::Max);
+ if (ResultWhenConstEqualsOther != ResultWhenConstNeOther)
+ return false; // The comparison is not tautological.
+ } else if (ResultWhenConstEqualsOther == ConstIsLowerBound)
return false; // The comparison is not tautological.
const bool Result = ResultWhenConstEqualsOther;