[clang][diagnostics] added warning for possible enum compare typo (#168445)

Added diagnosis and fixit comment for possible accidental comparison
operator in an enum.

Closes: #168146

Author: GrumpyPigSkin

clang/docs/ReleaseNotes.rst

@@ -441,6 +441,10 @@ Improvements to Clang's diagnostics
 - Clang no longer emits ``-Wmissing-noreturn`` for virtual methods where
   the function body consists of a `throw` expression (#GH167247).
 
+- A new warning ``-Wenum-compare-typo`` has been added to detect potential erroneous
+  comparison operators when mixed with bitwise operators in enum value initializers.
+  This can be locally disabled by explicitly casting the initializer value.
+
 Improvements to Clang's time-trace
 ----------------------------------
 

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -13739,4 +13739,12 @@ def err_amdgcn_load_lds_size_invalid_value : Error<"invalid size value">;
 def note_amdgcn_load_lds_size_valid_value : Note<"size must be %select{1, 2, or 4|1, 2, 4, 12 or 16}0">;
 
 def err_amdgcn_coop_atomic_invalid_as : Error<"cooperative atomic requires a global or generic pointer">;
+
+def warn_comparison_in_enum_initializer : Warning<
+  "comparison operator '%0' is potentially a typo for a shift operator '%1'">,
+  InGroup<DiagGroup<"enum-compare-typo">>;
+
+def note_enum_compare_typo_suggest : Note<
+  "use '%0' to perform a bitwise shift">;
+
 } // end of sema component.

clang/lib/Sema/SemaDecl.cpp

@@ -63,6 +63,7 @@
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/SaveAndRestore.h"
 #include "llvm/TargetParser/Triple.h"
 #include <algorithm>
@@ -20610,6 +20611,59 @@ bool Sema::IsValueInFlagEnum(const EnumDecl *ED, const llvm::APInt &Val,
   return !(FlagMask & Val) || (AllowMask && !(FlagMask & ~Val));
 }
 
+// Emits a warning when a suspicious comparison operator is used along side
+// binary operators in enum initializers.
+static void CheckForComparisonInEnumInitializer(SemaBase &Sema,
+                                                const EnumDecl *Enum) {
+  bool HasBitwiseOp = false;
+  SmallVector<const BinaryOperator *, 4> SuspiciousCompares;
+
+  // Iterate over all the enum values, gather suspisious comparison ops and
+  // whether any enum initialisers contain a binary operator.
+  for (const auto *ECD : Enum->enumerators()) {
+    const Expr *InitExpr = ECD->getInitExpr();
+    if (!InitExpr)
+      continue;
+
+    const Expr *E = InitExpr->IgnoreParenImpCasts();
+
+    if (const auto *BinOp = dyn_cast<BinaryOperator>(E)) {
+      BinaryOperatorKind Op = BinOp->getOpcode();
+
+      // Check for bitwise ops (<<, >>, &, |)
+      if (BinOp->isBitwiseOp() || BinOp->isShiftOp()) {
+        HasBitwiseOp = true;
+      } else if (Op == BO_LT || Op == BO_GT) {
+        // Check for the typo pattern (Comparison < or >)
+        const Expr *LHS = BinOp->getLHS()->IgnoreParenImpCasts();
+        if (const auto *IntLiteral = dyn_cast<IntegerLiteral>(LHS)) {
+          // Specifically looking for accidental bitshifts "1 < X" or "1 > X"
+          if (IntLiteral->getValue() == 1)
+            SuspiciousCompares.push_back(BinOp);
+        }
+      }
+    }
+  }
+
+  // If we found a bitwise op and some sus compares, iterate over the compares
+  // and warn.
+  if (HasBitwiseOp) {
+    for (const auto *BinOp : SuspiciousCompares) {
+      StringRef SuggestedOp = (BinOp->getOpcode() == BO_LT)
+                                  ? BinaryOperator::getOpcodeStr(BO_Shl)
+                                  : BinaryOperator::getOpcodeStr(BO_Shr);
+      SourceLocation OperatorLoc = BinOp->getOperatorLoc();
+
+      Sema.Diag(OperatorLoc, diag::warn_comparison_in_enum_initializer)
+          << BinOp->getOpcodeStr() << SuggestedOp;
+
+      Sema.Diag(OperatorLoc, diag::note_enum_compare_typo_suggest)
+          << SuggestedOp
+          << FixItHint::CreateReplacement(OperatorLoc, SuggestedOp);
+    }
+  }
+}
+
 void Sema::ActOnEnumBody(SourceLocation EnumLoc, SourceRange BraceRange,
                          Decl *EnumDeclX, ArrayRef<Decl *> Elements, Scope *S,
                          const ParsedAttributesView &Attrs) {
@@ -20747,6 +20801,7 @@ void Sema::ActOnEnumBody(SourceLocation EnumLoc, SourceRange BraceRange,
                            NumPositiveBits, NumNegativeBits);
 
   CheckForDuplicateEnumValues(*this, Elements, Enum, EnumType);
+  CheckForComparisonInEnumInitializer(*this, Enum);
 
   if (Enum->isClosedFlag()) {
     for (Decl *D : Elements) {

clang/test/Sema/warn-enum-compare-typo.c

@@ -0,0 +1,98 @@
+// RUN: %clang_cc1 -fsyntax-only -Wenum-compare-typo -verify %s 
+// RUN: %clang_cc1 -fsyntax-only -Wenum-compare-typo -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+
+enum PossibleTypoLeft { 
+  Val1 = 1 << 0,
+  // expected-warning@+3 {{comparison operator '<' is potentially a typo for a shift operator '<<'}} 
+  // expected-note@+2 {{use '<<' to perform a bitwise shift}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE+1]]:12-[[@LINE+1]]:13}:"<<"
+  Bad1 = 1 < 2, 
+  // expected-warning@+3 {{comparison operator '>' is potentially a typo for a shift operator '>>'}} 
+  // expected-note@+2 {{use '>>' to perform a bitwise shift}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE+1]]:12-[[@LINE+1]]:13}:">>"
+  Bad2 = 1 > 3,
+  // expected-warning@+3 {{comparison operator '>' is potentially a typo for a shift operator '>>'}} 
+  // expected-note@+2 {{use '>>' to perform a bitwise shift}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE+1]]:13-[[@LINE+1]]:14}:">>"
+  Bad3 = (1 > 3) 
+};
+
+enum PossibleTypoRight { 
+  Val2 = 1 >> 0,
+  // expected-warning@+3 {{comparison operator '<' is potentially a typo for a shift operator '<<'}} 
+  // expected-note@+2 {{use '<<' to perform a bitwise shift}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE+1]]:12-[[@LINE+1]]:13}:"<<"
+  Bad4 = 1 < 2, 
+  // expected-warning@+3 {{comparison operator '>' is potentially a typo for a shift operator '>>'}} 
+  // expected-note@+2 {{use '>>' to perform a bitwise shift}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE+1]]:12-[[@LINE+1]]:13}:">>"
+  Bad5 = 1 > 3,
+  // expected-warning@+3 {{comparison operator '<' is potentially a typo for a shift operator '<<'}} 
+  // expected-note@+2 {{use '<<' to perform a bitwise shift}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE+1]]:13-[[@LINE+1]]:14}:"<<"
+  Bad6 = (1 < 3) 
+};
+
+// Case 3: Context provided by other bitwise operators (&, |)
+// Even though there are no shifts, the presence of '|' implies flags.
+enum PossibleTypoBitwiseOr {
+  FlagA = 0x1,
+  FlagB = 0x2,
+  FlagCombo = FlagA | FlagB,
+  // expected-warning@+3 {{comparison operator '<' is potentially a typo for a shift operator '<<'}} 
+  // expected-note@+2 {{use '<<' to perform a bitwise shift}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE+1]]:17-[[@LINE+1]]:18}:"<<"
+  FlagTypo1 = 1 < FlagCombo,
+  // expected-warning@+3 {{comparison operator '>' is potentially a typo for a shift operator '>>'}} 
+  // expected-note@+2 {{use '>>' to perform a bitwise shift}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE+1]]:17-[[@LINE+1]]:18}:">>"
+  FlagTypo2 = 1 > FlagCombo
+};
+
+enum PossibleTypoBitwiseAnd {
+  FlagAnd = FlagA & FlagB,
+  // expected-warning@+3 {{comparison operator '<' is potentially a typo for a shift operator '<<'}} 
+  // expected-note@+2 {{use '<<' to perform a bitwise shift}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE+1]]:17-[[@LINE+1]]:18}:"<<"
+  FlagTypo3 = 1 < FlagAnd,
+  // expected-warning@+3 {{comparison operator '>' is potentially a typo for a shift operator '>>'}} 
+  // expected-note@+2 {{use '>>' to perform a bitwise shift}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE+1]]:17-[[@LINE+1]]:18}:">>"
+  FlagTypo4 = 1 > FlagAnd
+};
+
+enum NoWarningOnDirectInit {
+  A = 0,
+  B = 1,
+  Ok1 = 1 < 2, // No warning expected
+  Ok2 = 1 > 2 // No warning expected
+};
+
+enum NoWarningOnArith {
+  D = 0 + 1,
+  E = D * 10,
+  F = E - D,
+  G = F / D,
+  Ok3 = 1 < E, // No warning expected
+  Ok4 = 1 > E // No warning expected
+};
+
+enum NoWarningOnExplicitCast {
+  Bit1 = 1 << 0,
+  Ok5 = (int)(1 < 2) // No warning expected
+};
+
+enum NoWarningOnNoneBitShift {
+  Bit2 = 1 << 0,
+  Ok6 = (3 < 2) // No warning expected
+};
+
+// Ensure the diagnostic group works
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wenum-compare-typo"
+enum IGNORED {
+  Ok7 = 1 << 1,
+  Ignored3 = 1 < 10 // No warning
+};
+#pragma clang diagnostic pop