[Diagnostics] Warn if enumeration type mismatch in conditional expression

Summary:
- Useful warning
- GCC compatibility (GCC warns in C++ mode)

Reviewers: rsmith, aaron.ballman

Reviewed By: aaron.ballman

Subscribers: cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D67919

llvm-svn: 373252

2 files changed, 67 insertions, 0 deletions

diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 28f4e06cd72..7c37932058e 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -11308,6 +11308,32 @@ static const IntegerLiteral *getIntegerLiteral(Expr *E) {
   return IL;
 }
 
+static void CheckConditionalWithEnumTypes(Sema &S, SourceLocation Loc,
+                                          Expr *LHS, Expr *RHS) {
+  QualType LHSStrippedType = LHS->IgnoreParenImpCasts()->getType();
+  QualType RHSStrippedType = RHS->IgnoreParenImpCasts()->getType();
+
+  const auto *LHSEnumType = LHSStrippedType->getAs<EnumType>();
+  if (!LHSEnumType)
+    return;
+  const auto *RHSEnumType = RHSStrippedType->getAs<EnumType>();
+  if (!RHSEnumType)
+    return;
+
+  // Ignore anonymous enums.
+  if (!LHSEnumType->getDecl()->hasNameForLinkage())
+    return;
+  if (!RHSEnumType->getDecl()->hasNameForLinkage())
+    return;
+
+  if (S.Context.hasSameUnqualifiedType(LHSStrippedType, RHSStrippedType))
+    return;
+
+  S.Diag(Loc, diag::warn_conditional_mixed_enum_types)
+      << LHSStrippedType << RHSStrippedType << LHS->getSourceRange()
+      << RHS->getSourceRange();
+}
+
 static void DiagnoseIntInBoolContext(Sema &S, Expr *E) {
   E = E->IgnoreParenImpCasts();
   SourceLocation ExprLoc = E->getExprLoc();
@@ -11799,6 +11825,8 @@ static void CheckConditionalOperator(Sema &S, ConditionalOperator *E,
   bool Suspicious = false;
   CheckConditionalOperand(S, E->getTrueExpr(), T, CC, Suspicious);
   CheckConditionalOperand(S, E->getFalseExpr(), T, CC, Suspicious);
+  CheckConditionalWithEnumTypes(S, E->getBeginLoc(), E->getTrueExpr(),
+                                E->getFalseExpr());
 
   if (T->isBooleanType())
     DiagnoseIntInBoolContext(S, E);
diff --git a/clang/test/Sema/warn-conditional-emum-types-mismatch.c b/clang/test/Sema/warn-conditional-emum-types-mismatch.c
new file mode 100644
index 00000000000..4325bb0c41c
--- /dev/null
+++ b/clang/test/Sema/warn-conditional-emum-types-mismatch.c
@@ -0,0 +1,39 @@
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wenum-compare %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify  %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wenum-compare %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify %s
+
+enum ro { A = 0x10 };
+enum rw { B = 0xFF };
+enum { C = 0x1A};
+
+enum {
+  STATUS_SUCCESS,
+  STATUS_FAILURE,
+  MAX_BASE_STATUS_CODE
+};
+
+enum ExtendedStatusCodes {
+  STATUS_SOMETHING_INTERESTING = MAX_BASE_STATUS_CODE + 1000,
+};
+
+
+int get_flag(int cond) {
+  return cond ? A : B; 
+  #ifdef __cplusplus
+  // expected-warning@-2 {{enumeration type mismatch in conditional expression ('ro' and 'rw')}}
+  #else 
+  // expected-no-diagnostics
+  #endif
+}
+
+// In the following cases we purposefully differ from GCC and dont warn because
+// this code pattern is quite sensitive and we dont want to produce so many false positives.
+
+int get_flag_anon_enum(int cond) {
+  return cond ? A : C;
+}
+
+int foo(int c) {
+  return c ? STATUS_SOMETHING_INTERESTING : STATUS_SUCCESS;
+}