Author: GrumpyPigSkin Date: 2025-11-20T09:05:03-05:00 New Revision: aeba7a8adef9a8ea624aa5be3b95a8a9fb5a17dd
URL: https://github.com/llvm/llvm-project/commit/aeba7a8adef9a8ea624aa5be3b95a8a9fb5a17dd DIFF: https://github.com/llvm/llvm-project/commit/aeba7a8adef9a8ea624aa5be3b95a8a9fb5a17dd.diff LOG: [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 Added: clang/test/Sema/warn-enum-compare-typo.c Modified: clang/docs/ReleaseNotes.rst clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/Sema/SemaDecl.cpp Removed: ################################################################################ diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 0746fa93209d3..c80e060f6b7d2 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/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 ---------------------------------- diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 533bebca2fe17..53aa86a7dabde 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/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. diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index b7aecadc86871..b8ca2a376fde8 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/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) { diff --git a/clang/test/Sema/warn-enum-compare-typo.c b/clang/test/Sema/warn-enum-compare-typo.c new file mode 100644 index 0000000000000..f937df1e6cd87 --- /dev/null +++ b/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 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
