Index: include/clang/AST/Expr.h =================================================================== --- include/clang/AST/Expr.h +++ include/clang/AST/Expr.h @@ -906,6 +906,11 @@ return skipRValueSubobjectAdjustments(CommaLHSs, Adjustments); } + /// Checks that the two Expr's will refer to the same value as a comparison + /// operand. The caller must ensure that the values referenced by the Expr's + /// are not modified between E1 and E2 or the result my be invalid. + static bool isSameComparisonOperand(const Expr* E1, const Expr* E2); + static bool classof(const Stmt *T) { return T->getStmtClass() >= firstExprConstant && T->getStmtClass() <= lastExprConstant; Index: include/clang/Analysis/CFG.h =================================================================== --- include/clang/Analysis/CFG.h +++ include/clang/Analysis/CFG.h @@ -1023,6 +1023,7 @@ virtual void compareAlwaysTrue(const BinaryOperator *B, bool isAlwaysTrue) {} virtual void compareBitwiseEquality(const BinaryOperator *B, bool isAlwaysTrue) {} + virtual void compareBitwiseOr(const BinaryOperator *B) {} }; /// Represents a source-level, intra-procedural CFG that represents the Index: include/clang/Basic/DiagnosticGroups.td =================================================================== --- include/clang/Basic/DiagnosticGroups.td +++ include/clang/Basic/DiagnosticGroups.td @@ -498,12 +498,14 @@ [TautologicalOutOfRangeCompare]>; def TautologicalPointerCompare : DiagGroup<"tautological-pointer-compare">; def TautologicalOverlapCompare : DiagGroup<"tautological-overlap-compare">; +def TautologicalBitwiseCompare : DiagGroup<"tautological-bitwise-compare">; def TautologicalUndefinedCompare : DiagGroup<"tautological-undefined-compare">; def TautologicalObjCBoolCompare : DiagGroup<"tautological-objc-bool-compare">; def TautologicalCompare : DiagGroup<"tautological-compare", [TautologicalConstantCompare, TautologicalPointerCompare, TautologicalOverlapCompare, + TautologicalBitwiseCompare, TautologicalUndefinedCompare, TautologicalObjCBoolCompare]>; def HeaderHygiene : DiagGroup<"header-hygiene">; Index: include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -8155,7 +8155,10 @@ InGroup; def warn_comparison_bitwise_always : Warning< "bitwise comparison always evaluates to %select{false|true}0">, - InGroup; + InGroup, DefaultIgnore; +def warn_comparison_bitwise_or : Warning< + "bitwise or with non-zero value always evaluates to true">, + InGroup, DefaultIgnore; def warn_tautological_overlap_comparison : Warning< "overlapping comparisons always evaluate to %select{false|true}0">, InGroup, DefaultIgnore; Index: lib/AST/Expr.cpp =================================================================== --- lib/AST/Expr.cpp +++ lib/AST/Expr.cpp @@ -3915,6 +3915,111 @@ return false; } +bool Expr::isSameComparisonOperand(const Expr* E1, const Expr* E2) { + E1 = E1->IgnoreParens(); + E2 = E2->IgnoreParens(); + + if (E1->getStmtClass() != E2->getStmtClass()) + return false; + + switch (E1->getStmtClass()) { + default: + return false; + case CXXThisExprClass: + return true; + case DeclRefExprClass: { + // DeclRefExpr without an ImplicitCastExpr can happen for integral + // template parameters. + const auto *DRE1 = cast(E1); + const auto *DRE2 = cast(E2); + return DRE1->isRValue() && DRE2->isRValue() && + DRE1->getDecl() == DRE2->getDecl(); + } + case ImplicitCastExprClass: { + // Peel off implicit casts. + while (true) { + const auto *ICE1 = dyn_cast(E1); + const auto *ICE2 = dyn_cast(E2); + if (!ICE1 || !ICE2) + return false; + if (ICE1->getCastKind() != ICE2->getCastKind()) + return false; + E1 = ICE1->getSubExpr()->IgnoreParens(); + E2 = ICE2->getSubExpr()->IgnoreParens(); + // The final cast must be one of these types. + if (ICE1->getCastKind() == CK_LValueToRValue || + ICE1->getCastKind() == CK_ArrayToPointerDecay || + ICE1->getCastKind() == CK_FunctionToPointerDecay) { + break; + } + } + + const auto *DRE1 = dyn_cast(E1); + const auto *DRE2 = dyn_cast(E2); + if (DRE1 && DRE2) + return declaresSameEntity(DRE1->getDecl(), DRE2->getDecl()); + + const auto *Ivar1 = dyn_cast(E1); + const auto *Ivar2 = dyn_cast(E2); + if (Ivar1 && Ivar2) { + return Ivar1->isFreeIvar() && Ivar2->isFreeIvar() && + declaresSameEntity(Ivar1->getDecl(), Ivar2->getDecl()); + } + + const auto *Array1 = dyn_cast(E1); + const auto *Array2 = dyn_cast(E2); + if (Array1 && Array2) { + if (!isSameComparisonOperand(Array1->getBase(), Array2->getBase())) + return false; + + auto Idx1 = Array1->getIdx(); + auto Idx2 = Array2->getIdx(); + const auto Integer1 = dyn_cast(Idx1); + const auto Integer2 = dyn_cast(Idx2); + if (Integer1 && Integer2) { + if (Integer1->getValue() != Integer2->getValue()) + return false; + } else { + if (!isSameComparisonOperand(Idx1, Idx2)) + return false; + } + + return true; + } + + // Walk the MemberExpr chain. + while (isa(E1) && isa(E2)) { + const auto *ME1 = cast(E1); + const auto *ME2 = cast(E2); + if (!declaresSameEntity(ME1->getMemberDecl(), ME2->getMemberDecl())) + return false; + if (const auto *D = dyn_cast(ME1->getMemberDecl())) + if (D->isStaticDataMember()) + return true; + E1 = ME1->getBase()->IgnoreParenImpCasts(); + E2 = ME2->getBase()->IgnoreParenImpCasts(); + } + + if (isa(E1) && isa(E2)) + return true; + + // A static member variable can end the MemberExpr chain with either + // a MemberExpr or a DeclRefExpr. + auto getAnyDecl = [](const Expr *E) -> const ValueDecl * { + if (auto *DRE = dyn_cast(E)) + return DRE->getDecl(); + if (auto *ME = dyn_cast(E)) + return ME->getMemberDecl(); + return nullptr; + }; + + const ValueDecl *VD1 = getAnyDecl(E1); + const ValueDecl *VD2 = getAnyDecl(E2); + return declaresSameEntity(VD1, VD2); + } + } +} + /// isArrow - Return true if the base expression is a pointer to vector, /// return false if the base expression is a vector. bool ExtVectorElementExpr::isArrow() const { Index: lib/Analysis/CFG.cpp =================================================================== --- lib/Analysis/CFG.cpp +++ lib/Analysis/CFG.cpp @@ -70,23 +70,61 @@ return D->getLocation(); } +/// Returns true on constant values based around a single IntegerLiteral. +/// Allow for use of parentheses, integer casts, and negative signs. +static bool IsIntegerLiteralConstantExpr(const Expr *E) { + // Allow parentheses + E = E->IgnoreParens(); + + // Allow conversions to different integer kind. + if (const auto *CE = dyn_cast(E)) { + if (CE->getCastKind() != CK_IntegralCast) + return false; + E = CE->getSubExpr(); + } + + // Allow negative numbers. + if (const auto *UO = dyn_cast(E)) { + if (UO->getOpcode() != UO_Minus) + return false; + E = UO->getSubExpr(); + } + + return isa(E); +} + /// Helper for tryNormalizeBinaryOperator. Attempts to extract an IntegerLiteral -/// or EnumConstantDecl from the given Expr. If it fails, returns nullptr. +/// constant expression or EnumConstantDecl from the given Expr. If it fails, +/// returns nullptr. static const Expr *tryTransformToIntOrEnumConstant(const Expr *E) { E = E->IgnoreParens(); - if (isa(E)) + if (IsIntegerLiteralConstantExpr(E)) return E; if (auto *DR = dyn_cast(E->IgnoreParenImpCasts())) return isa(DR->getDecl()) ? DR : nullptr; return nullptr; } -/// Tries to interpret a binary operator into `Decl Op Expr` form, if Expr is -/// an integer literal or an enum constant. +/// Takes an expression returned from tryTransformToIntOrEnumConstant and +/// evaluates its non-zeroness. +static bool isIntOrEnumConstantZero(const Expr *E) { + E = E->IgnoreParens(); + if (const auto *Integer = dyn_cast(E)) + return Integer->getValue() == 0; + + // These are non-null because they have been checked in + // tryTransformToIntOrEnumConstant + const auto *DR = cast(E->IgnoreParenImpCasts()); + const auto *EC = cast(DR->getDecl()); + return EC->getInitVal() == 0; +} + +/// Tries to interpret a binary operator into `Expr Op NumExpr` form, if +/// NumExpr is an integer literal or an enum constant. /// /// If this fails, at least one of the returned DeclRefExpr or Expr will be /// null. -static std::tuple +static std::tuple tryNormalizeBinaryOperator(const BinaryOperator *B) { BinaryOperatorKind Op = B->getOpcode(); @@ -108,8 +146,7 @@ Constant = tryTransformToIntOrEnumConstant(B->getLHS()); } - auto *D = dyn_cast(MaybeDecl->IgnoreParenImpCasts()); - return std::make_tuple(D, Op, Constant); + return std::make_tuple(MaybeDecl, Op, Constant); } /// For an expression `x == Foo && x == Bar`, this determines whether the @@ -121,11 +158,11 @@ static bool areExprTypesCompatible(const Expr *E1, const Expr *E2) { // User intent isn't clear if they're mixing int literals with enum // constants. - if (isa(E1) != isa(E2)) + if (isa(E1) != isa(E2)) return false; // Integer literal comparisons, regardless of literal type, are acceptable. - if (isa(E1)) + if (!isa(E1)) return true; // IntegerLiterals are handled above and only EnumConstantDecls are expected @@ -1017,34 +1054,34 @@ if (!LHS->isComparisonOp() || !RHS->isComparisonOp()) return {}; - const DeclRefExpr *Decl1; - const Expr *Expr1; + const Expr *DeclExpr1; + const Expr *NumExpr1; BinaryOperatorKind BO1; - std::tie(Decl1, BO1, Expr1) = tryNormalizeBinaryOperator(LHS); + std::tie(DeclExpr1, BO1, NumExpr1) = tryNormalizeBinaryOperator(LHS); - if (!Decl1 || !Expr1) + if (!DeclExpr1 || !NumExpr1) return {}; - const DeclRefExpr *Decl2; - const Expr *Expr2; + const Expr *DeclExpr2; + const Expr *NumExpr2; BinaryOperatorKind BO2; - std::tie(Decl2, BO2, Expr2) = tryNormalizeBinaryOperator(RHS); + std::tie(DeclExpr2, BO2, NumExpr2) = tryNormalizeBinaryOperator(RHS); - if (!Decl2 || !Expr2) + if (!DeclExpr2 || !NumExpr2) return {}; // Check that it is the same variable on both sides. - if (Decl1->getDecl() != Decl2->getDecl()) + if (!Expr::isSameComparisonOperand(DeclExpr1, DeclExpr2)) return {}; // Make sure the user's intent is clear (e.g. they're comparing against two // int literals, or two things from the same enum) - if (!areExprTypesCompatible(Expr1, Expr2)) + if (!areExprTypesCompatible(NumExpr1, NumExpr2)) return {}; Expr::EvalResult L1Result, L2Result; - if (!Expr1->EvaluateAsInt(L1Result, *Context) || - !Expr2->EvaluateAsInt(L2Result, *Context)) + if (!NumExpr1->EvaluateAsInt(L1Result, *Context) || + !NumExpr2->EvaluateAsInt(L2Result, *Context)) return {}; llvm::APSInt L1 = L1Result.Val.getInt(); @@ -1077,6 +1114,10 @@ // * Variable x is equal to the largest literal. // * Variable x is greater than largest literal. bool AlwaysTrue = true, AlwaysFalse = true; + // Track value of both subexpression. If either side is always + // true/false, another warning should have already been emitted. + bool LHSAlwaysTrue = true, LHSAlwaysFalse = true; + bool RHSAlwaysTrue = true, RHSAlwaysFalse = true; for (const llvm::APSInt &Value : Values) { TryResult Res1, Res2; Res1 = analyzeLogicOperatorCondition(BO1, Value, L1); @@ -1092,16 +1133,43 @@ AlwaysTrue &= (Res1.isTrue() || Res2.isTrue()); AlwaysFalse &= !(Res1.isTrue() || Res2.isTrue()); } + + LHSAlwaysTrue &= Res1.isTrue(); + LHSAlwaysFalse &= Res1.isFalse(); + RHSAlwaysTrue &= Res2.isTrue(); + RHSAlwaysFalse &= Res2.isFalse(); } if (AlwaysTrue || AlwaysFalse) { - if (BuildOpts.Observer) + if (!LHSAlwaysTrue && !LHSAlwaysFalse && !RHSAlwaysTrue && + !RHSAlwaysFalse && BuildOpts.Observer) BuildOpts.Observer->compareAlwaysTrue(B, AlwaysTrue); return TryResult(AlwaysTrue); } return {}; } + /// A bitwise-or with a non-zero constant always evaluates to true. + TryResult checkIncorrectBitwiseOrOperator(const BinaryOperator *B) { + const Expr *LHSConstant = + tryTransformToIntOrEnumConstant(B->getLHS()->IgnoreParenImpCasts()); + const Expr *RHSConstant = + tryTransformToIntOrEnumConstant(B->getRHS()->IgnoreParenImpCasts()); + + if ((LHSConstant && RHSConstant) || (!LHSConstant && !RHSConstant)) + return {}; + + const Expr *Constant = LHSConstant ? LHSConstant : RHSConstant; + + if (isIntOrEnumConstantZero(Constant)) + return {}; + + if (BuildOpts.Observer) + BuildOpts.Observer->compareBitwiseOr(B); + + return TryResult(true); + } + /// Try and evaluate an expression to an integer constant. bool tryEvaluate(Expr *S, Expr::EvalResult &outResult) { if (!BuildOpts.PruneTriviallyFalseEdges) @@ -1119,7 +1187,7 @@ return {}; if (BinaryOperator *Bop = dyn_cast(S)) { - if (Bop->isLogicalOp()) { + if (Bop->isLogicalOp() || Bop->isEqualityOp()) { // Check the cache first. CachedBoolEvalsTy::iterator I = CachedBoolEvals.find(S); if (I != CachedBoolEvals.end()) @@ -1203,6 +1271,10 @@ TryResult BopRes = checkIncorrectRelationalOperator(Bop); if (BopRes.isKnown()) return BopRes.isTrue(); + } else if (Bop->getOpcode() == BO_Or) { + TryResult BopRes = checkIncorrectBitwiseOrOperator(Bop); + if (BopRes.isKnown()) + return BopRes.isTrue(); } } @@ -2301,6 +2373,9 @@ appendStmt(Block, U); } + if (U->getOpcode() == UO_LNot) + tryEvaluateBool(U->getSubExpr()->IgnoreParens()); + return Visit(U->getSubExpr(), AddStmtChoice()); } @@ -2435,6 +2510,9 @@ appendStmt(Block, B); } + if (B->isEqualityOp() || B->isRelationalOp()) + tryEvaluateBool(B); + CFGBlock *RBlock = Visit(B->getRHS()); CFGBlock *LBlock = Visit(B->getLHS()); // If visiting RHS causes us to finish 'Block', e.g. the RHS is a StmtExpr @@ -4477,6 +4555,10 @@ autoCreateBlock(); appendStmt(Block, E); } + + if (E->getCastKind() == CK_IntegralToBoolean) + tryEvaluateBool(E->getSubExpr()->IgnoreParens()); + return Visit(E->getSubExpr(), AddStmtChoice()); } Index: lib/Analysis/ReachableCode.cpp =================================================================== --- lib/Analysis/ReachableCode.cpp +++ lib/Analysis/ReachableCode.cpp @@ -247,7 +247,7 @@ } case Stmt::UnaryOperatorClass: { const UnaryOperator *UO = cast(S); - if (UO->getOpcode() != UO_LNot) + if (UO->getOpcode() != UO_LNot && UO->getOpcode() != UO_Minus) return false; bool SilenceableCondValNotSet = SilenceableCondVal && SilenceableCondVal->getBegin().isInvalid(); Index: lib/Sema/AnalysisBasedWarnings.cpp =================================================================== --- lib/Sema/AnalysisBasedWarnings.cpp +++ lib/Sema/AnalysisBasedWarnings.cpp @@ -159,6 +159,20 @@ S.Diag(B->getExprLoc(), diag::warn_comparison_bitwise_always) << DiagRange << isAlwaysTrue; } + + void compareBitwiseOr(const BinaryOperator *B) override { + if (HasMacroID(B)) + return; + + SourceRange DiagRange = B->getSourceRange(); + S.Diag(B->getExprLoc(), diag::warn_comparison_bitwise_or) << DiagRange; + } + + static bool hasActiveDiagnostics(DiagnosticsEngine &Diags, + SourceLocation Loc) { + return !Diags.isIgnored(diag::warn_tautological_overlap_comparison, Loc) || + !Diags.isIgnored(diag::warn_comparison_bitwise_or, Loc); + } }; } // anonymous namespace @@ -2076,10 +2090,9 @@ .setAlwaysAdd(Stmt::AttributedStmtClass); } - // Install the logical handler for -Wtautological-overlap-compare + // Install the logical handler. llvm::Optional LEH; - if (!Diags.isIgnored(diag::warn_tautological_overlap_comparison, - D->getBeginLoc())) { + if (LogicalErrorHandler::hasActiveDiagnostics(Diags, D->getBeginLoc())) { LEH.emplace(S); AC.getCFGBuildOptions().Observer = &*LEH; } @@ -2228,9 +2241,8 @@ checkThrowInNonThrowingFunc(S, FD, AC); // If none of the previous checks caused a CFG build, trigger one here - // for -Wtautological-overlap-compare - if (!Diags.isIgnored(diag::warn_tautological_overlap_comparison, - D->getBeginLoc())) { + // for the logical error handler. + if (LogicalErrorHandler::hasActiveDiagnostics(Diags, D->getBeginLoc())) { AC.getCFG(); } Index: lib/Sema/SemaExpr.cpp =================================================================== --- lib/Sema/SemaExpr.cpp +++ lib/Sema/SemaExpr.cpp @@ -7490,7 +7490,12 @@ static bool IsArithmeticOp(BinaryOperatorKind Opc) { return BinaryOperator::isAdditiveOp(Opc) || BinaryOperator::isMultiplicativeOp(Opc) || - BinaryOperator::isShiftOp(Opc); + BinaryOperator::isShiftOp(Opc) || Opc == BO_And || Opc == BO_Or; + // This only checks for bitwise-or and bitwise-and, but not bitwise-xor and + // not any of the logical operators. Bitwise-xor is commonly used as a + // logical-xor because there is no logical-xor operator. The logical + // operators, including uses of xor, have a high false positive rate for + // precedence warnings. } /// IsArithmeticBinaryExpr - Returns true if E is an arithmetic binary @@ -10131,20 +10136,18 @@ << FixItHint::CreateInsertion(SecondClose, ")"); } -// Get the decl for a simple expression: a reference to a variable, -// an implicit C++ field reference, or an implicit ObjC ivar reference. -static ValueDecl *getCompareDecl(Expr *E) { - if (DeclRefExpr *DR = dyn_cast(E)) - return DR->getDecl(); - if (ObjCIvarRefExpr *Ivar = dyn_cast(E)) { - if (Ivar->isFreeIvar()) - return Ivar->getDecl(); - } - if (MemberExpr *Mem = dyn_cast(E)) { +// Returns true if E refers to a non-weak array. +static bool checkForArray(const Expr *E) { + const ValueDecl *D = nullptr; + if (const DeclRefExpr *DR = dyn_cast(E)) { + D = DR->getDecl(); + } else if (const MemberExpr *Mem = dyn_cast(E)) { if (Mem->isImplicitAccess()) - return Mem->getMemberDecl(); + D = Mem->getMemberDecl(); } - return nullptr; + if (!D) + return false; + return D->getType()->isArrayType() && !D->isWeak(); } /// Diagnose some forms of syntactically-obvious tautological comparison. @@ -10177,8 +10180,6 @@ // obvious cases in the definition of the template anyways. The idea is to // warn when the typed comparison operator will always evaluate to the same // result. - ValueDecl *DL = getCompareDecl(LHSStripped); - ValueDecl *DR = getCompareDecl(RHSStripped); // Used for indexing into %select in warn_comparison_always enum { @@ -10187,7 +10188,8 @@ AlwaysFalse, AlwaysEqual, // std::strong_ordering::equal from operator<=> }; - if (DL && DR && declaresSameEntity(DL, DR)) { + + if (Expr::isSameComparisonOperand(LHS, RHS)) { unsigned Result; switch (Opc) { case BO_EQ: case BO_LE: case BO_GE: @@ -10207,9 +10209,7 @@ S.PDiag(diag::warn_comparison_always) << 0 /*self-comparison*/ << Result); - } else if (DL && DR && - DL->getType()->isArrayType() && DR->getType()->isArrayType() && - !DL->isWeak() && !DR->isWeak()) { + } else if (checkForArray(LHSStripped) && checkForArray(RHSStripped)) { // What is it always going to evaluate to? unsigned Result; switch(Opc) { Index: test/Analysis/array-struct-region.cpp =================================================================== --- test/Analysis/array-struct-region.cpp +++ test/Analysis/array-struct-region.cpp @@ -1,20 +1,26 @@ // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\ // RUN: -analyzer-checker=debug.ExprInspection -verify\ +// RUN: -Wno-tautological-compare\ // RUN: -x c %s // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\ // RUN: -analyzer-checker=debug.ExprInspection -verify\ +// RUN: -Wno-tautological-compare\ // RUN: -x c++ -std=c++14 %s // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\ // RUN: -analyzer-checker=debug.ExprInspection -verify\ +// RUN: -Wno-tautological-compare\ // RUN: -x c++ -std=c++17 %s // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\ // RUN: -analyzer-checker=debug.ExprInspection -verify\ +// RUN: -Wno-tautological-compare\ // RUN: -DINLINE -x c %s // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\ // RUN: -analyzer-checker=debug.ExprInspection -verify\ +// RUN: -Wno-tautological-compare\ // RUN: -DINLINE -x c++ -std=c++14 %s // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\ // RUN: -analyzer-checker=debug.ExprInspection -verify\ +// RUN: -Wno-tautological-compare\ // RUN: -DINLINE -x c++ -std=c++17 %s void clang_analyzer_eval(int); Index: test/Sema/parentheses.c =================================================================== --- test/Sema/parentheses.c +++ test/Sema/parentheses.c @@ -93,6 +93,23 @@ (void)(x + y > 0 ? 1 : 2); // no warning (void)(x + (y > 0) ? 1 : 2); // expected-warning {{operator '?:' has lower precedence than '+'}} expected-note 2{{place parentheses}} + + (void)(x | b ? 1 : 2); // expected-warning {{operator '?:' has lower precedence than '|'}} expected-note 2{{place parentheses}} + (void)(x & b ? 1 : 2); // expected-warning {{operator '?:' has lower precedence than '&'}} expected-note 2{{place parentheses}} + + (void)((x | b) ? 1 : 2); // no warning, has parentheses + (void)(x | (b ? 1 : 2)); // no warning, has parentheses + (void)((x & b) ? 1 : 2); // no warning, has parentheses + (void)(x & (b ? 1 : 2)); // no warning, has parentheses + + // Only warn on uses of the bitwise operators, and not the logical operators. + // The bitwise operators are more likely to be bugs while the logical + // operators are more likely to be used correctly. Since there is no + // explicit logical-xor operator, the bitwise-xor is commonly used instead. + // For this warning, treat the bitwise-xor as if it were a logical operator. + (void)(x ^ b ? 1 : 2); // no warning, ^ is often used as logical xor + (void)(x || b ? 1 : 2); // no warning, logical operator + (void)(x && b ? 1 : 2); // no warning, logical operator } // RUN: not %clang_cc1 -fsyntax-only -Wparentheses -Werror -fdiagnostics-show-option %s 2>&1 | FileCheck %s -check-prefix=CHECK-FLAG Index: test/Sema/warn-bitwise-compare.c =================================================================== --- test/Sema/warn-bitwise-compare.c +++ test/Sema/warn-bitwise-compare.c @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -fsyntax-only -verify -Wtautological-compare %s +// RUN: %clang_cc1 -fsyntax-only -verify -Wtautological-bitwise-compare %s #define mydefine 2 @@ -13,6 +13,9 @@ if ((x & 0x15) == 0x13) {} // expected-warning {{bitwise comparison always evaluates to false}} if ((0x23 | x) == 0x155){} // expected-warning {{bitwise comparison always evaluates to false}} + if (!!((8 & x) == 3)) {} // expected-warning {{bitwise comparison always evaluates to false}} + int y = ((8 & x) == 3) ? 1 : 2; // expected-warning {{bitwise comparison always evaluates to false}} + if ((x & 8) == 8) {} if ((x & 8) != 8) {} if ((x | 4) == 4) {} @@ -26,3 +29,9 @@ if ((x & mydefine) == 8) {} if ((x | mydefine) == 4) {} } + +void g(int x) { + if (x | 5) {} // expected-warning {{bitwise or with non-zero value always evaluates to true}} + if (5 | x) {} // expected-warning {{bitwise or with non-zero value always evaluates to true}} + if (!((x | 5))) {} // expected-warning {{bitwise or with non-zero value always evaluates to true}} +} Index: test/Sema/warn-overlap.c =================================================================== --- test/Sema/warn-overlap.c +++ test/Sema/warn-overlap.c @@ -141,3 +141,34 @@ return x < 1 || x != 0; // expected-warning@-1{{overlapping comparisons always evaluate to true}} } + +struct A { + int x; + int y; +}; + +int struct_test(struct A a) { + return a.x > 5 && a.y < 1; // no warning, different variables + + return a.x > 5 && a.x < 1; + // expected-warning@-1{{overlapping comparisons always evaluate to false}} + return a.y == 1 || a.y != 1; + // expected-warning@-1{{overlapping comparisons always evaluate to true}} +} + +int integer_conversion(unsigned x, int y) { + return x > 4 || x < 10; + // expected-warning@-1{{overlapping comparisons always evaluate to true}} + return y > 4u || y < 10u; + // expected-warning@-1{{overlapping comparisons always evaluate to true}} +} + +int negative_compare(int x) { + return x > -1 || x < 1; + // expected-warning@-1{{overlapping comparisons always evaluate to true}} +} + +int no_warning(unsigned x) { + return x >= 0 || x == 1; + // no warning since "x >= 0" is caught by a different tautological warning. +} Index: test/Sema/warn-unreachable.c =================================================================== --- test/Sema/warn-unreachable.c +++ test/Sema/warn-unreachable.c @@ -433,7 +433,7 @@ } void unaryOpNoFixit() { - if (- 1) + if (~ 1) return; // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]] unaryOpNoFixit(); // expected-warning {{code will never be executed}} } Index: test/SemaCXX/self-comparison.cpp =================================================================== --- test/SemaCXX/self-comparison.cpp +++ test/SemaCXX/self-comparison.cpp @@ -40,3 +40,71 @@ Y::n == Y::n; } template bool g(); // should not produce any warnings + +namespace member_tests { +struct B { + int field; + static int static_field; + int test(B b) { + return field == field; // expected-warning {{self-comparison always evaluates to true}} + return static_field == static_field; // expected-warning {{self-comparison always evaluates to true}} + return static_field == b.static_field; // expected-warning {{self-comparison always evaluates to true}} + return B::static_field == this->static_field; // expected-warning {{self-comparison always evaluates to true}} + return this == this; // expected-warning {{self-comparison always evaluates to true}} + + return field == b.field; + return this->field == b.field; + } +}; + +enum { + I0, + I1, + I2, +}; + +struct S { + int field; + static int static_field; + int array[4]; +}; + +struct T { + int field; + static int static_field; + int array[4]; + S s; +}; + +int struct_test(S s1, S s2, S *s3, T t) { + return s1.field == s1.field; // expected-warning {{self-comparison always evaluates to true}} + return s2.field == s2.field; // expected-warning {{self-comparison always evaluates to true}} + return s1.static_field == s2.static_field; // expected-warning {{self-comparison always evaluates to true}} + return S::static_field == s1.static_field; // expected-warning {{self-comparison always evaluates to true}} + return s1.array == s1.array; // expected-warning {{self-comparison always evaluates to true}} + return t.s.static_field == S::static_field; // expected-warning {{self-comparison always evaluates to true}} + return s3->field == s3->field; // expected-warning {{self-comparison always evaluates to true}} + return s3->static_field == S::static_field; // expected-warning {{self-comparison always evaluates to true}} + return s1.array[0] == s1.array[0]; // expected-warning {{self-comparison always evaluates to true}} + return s1.array[I1] == s1.array[I1]; // expected-warning {{self-comparison always evaluates to true}} + return s1.array[s2.array[0]] == s1.array[s2.array[0]]; // expected-warning {{self-comparison always evaluates to true}} + return s3->array[t.field] == s3->array[t.field]; // expected-warning {{self-comparison always evaluates to true}} + + // Try all operators + return t.field == t.field; // expected-warning {{self-comparison always evaluates to true}} + return t.field <= t.field; // expected-warning {{self-comparison always evaluates to true}} + return t.field >= t.field; // expected-warning {{self-comparison always evaluates to true}} + + return t.field != t.field; // expected-warning {{self-comparison always evaluates to false}} + return t.field < t.field; // expected-warning {{self-comparison always evaluates to false}} + return t.field > t.field; // expected-warning {{self-comparison always evaluates to false}} + + // no warning + return s1.field == s2.field; + return s2.array == s1.array; + return s2.array[0] == s1.array[0]; + return s1.array[I1] == s1.array[I2]; + + return s1.static_field == t.static_field; +}; +} // namespace member_tests Index: test/SemaCXX/warn-unreachable.cpp =================================================================== --- test/SemaCXX/warn-unreachable.cpp +++ test/SemaCXX/warn-unreachable.cpp @@ -393,6 +393,9 @@ else calledFun(); // expected-warning {{will never be executed}} + if (y == -1 && y != -1) // expected-note {{silence}} + calledFun(); // expected-warning {{will never be executed}} + // TODO: Extend warning to the following code: if (x < -1) calledFun(); @@ -408,6 +411,4 @@ else calledFun(); - if (y == -1 && y != -1) - calledFun(); }