[clang-tidy] Misc redundant expressions checker updated for confused operators

[barancsuk]

Summary

Redundant Expression Checker is updated to target expressions with suspicious operator usage.

The checker finds the following two cases, where operators are suspected to be mistaken for another operator, that is considered to be more effective in that particular expression.

1.) Left shift operator might be confused with right shift operator
The expression always evaluates to zero, because Y is shifted left with more bits that the leftmost effective (1) bit of 0xff.

Y = (Y << 8) & 0xff;

// Possible intended expression:
Y = (Y >> 8) & 0xff;

2.) Bitwise NOT mistaken for logical NOT
A logical not is applied to a bitwise operator expression with integer constant operands, then assigned to an int.

int K = !(1 | 2 | 4);

// Possible intended expression:
int K = ~(1 | 2 | 4);

[xazax.hun]

clang-tidy/misc/RedundantExpressionCheck.cpp

698

Are you sure you wanted hasDescendant here? Would has be sufficient?

1040

I think either all of the binds should be successful in your matcher expression or none of them. So I think this might better be transformed into an assert.

1045

The first check should be transformed into an assert.

1051

Same as above.

[barancsuk]

Address review comments

[xazax.hun]

clang-tidy/misc/RedundantExpressionCheck.cpp

1040

I think looking up these AST nodes could be postponed until they are needed, so in case an early return is triggered, no lookup necessary.

[barancsuk]

Postpone AST node lookup

[xazax.hun]

Could you make sure that we do not offer a fixit when the location of the negation (!) is from a macro expansion?
Also, it might be a good idea to have some tests where some of the types do not match.

For example:
Y = (Y << 8) & 0xff;
Where Y is a long.
And also tests where one of the constants are longs.

clang-tidy/misc/RedundantExpressionCheck.cpp

29

Include <cmath> instead.

1025

This snippet is too long, probably not clang formatted.

[barancsuk]

Add new tests for unmatched types.
Do not provide fixit for ! operator from macros.

[xazax.hun]

Otherwise, from a nit that is probably an artifact from a rebase it looks good to me. But please wait for one more reviewer to accept this just to be sure.

clang-tidy/misc/RedundantExpressionCheck.cpp

135–136

Is this change intended or an artifact from a rebase?

[aaron.ballman]

Some quick concerns (I'll give it a more thorough look when I have a spare moment).

clang-tidy/misc/RedundantExpressionCheck.cpp

29

Please keep the include list sorted appropriately.

1030

Extra preceding space in the diagnostic.

1061

Extra preceding space here as well.

[barancsuk]

Address review comments, note modifications in ReleaseNotes

[aaron.ballman]

clang-tidy/misc/RedundantExpressionCheck.cpp

1030–1031

How about: "ineffective logical negation operator used; did you mean '~'?"

1061–1062

Diagnostics should not be complete sentences with full stops. How about: "ineffective bitwise and operation; did you mean '>>'?"

One thing that's strange about this is that the diagnostic location is under the bitwise and rather than the shift operation. Is that intentional?

test/clang-tidy/misc-redundant-expression.cpp

710

Ineffective

[barancsuk]

Address review comments

[barancsuk]

clang-tidy/misc/RedundantExpressionCheck.cpp

135–136

This change should have been introduced in the Misc redundant expressions checker updated for overloaded operators patch, so it must be an artifact from a rebase.

1061–1062

The main idea behind the diagnostic placement was that the first part of the sentence refers to the "bitwise and operation" rather than the shift operation.
However, after the first part of the message the diagnostic also asks about the shift operator, so I will easily change the placement, if you think that it would help the user understand the message better.

[aaron.ballman]

clang-tidy/misc/RedundantExpressionCheck.cpp

1061–1062

It's odd because the ineffective part of the written operation is the &, but the presumed cause of it being ineffective is due to using << instead of >>. It might also be reasonable to presume the & is at fault and that the user meant to write | to flip some bits on. I wonder if the best approach is to highlight the & and drop the "did you mean" part, because it seems like multiple things could have been at fault.

What do you think?

[barancsuk]

Drop first part of diagnostic message

[barancsuk]

clang-tidy/misc/RedundantExpressionCheck.cpp

1061–1062

It is a good point, that the ineffectiveness of the expression is due to the interaction of two operators, and it is unclear which one of the two is to blame.

I agree that the best way to approach this is to simply omit the second part of the diagnostic message and highlight the AND operation.

[aaron.ballman]

clang-tidy/misc/RedundantExpressionCheck.cpp

1061

Drop the full-stop from the end of the diagnostic.

test/clang-tidy/misc-redundant-expression.cpp

712

When you correct the diagnostic, you'll need to update the test cases to match.

[barancsuk]

Remove full stop from end of diagnostic

[aaron.ballman]

LGTM!

[barancsuk]

Rebase on master

[barancsuk]

Rebase on master 2.0

[xazax.hun]

Hmm, it looks like these changes are already upstreamed. Probably as part of other patch or maybe just someone (probably me) forgot to put the proper revision in the commit message? I close this revision now.

clang-tidy/misc/RedundantExpressionCheck.cpp

 #include "clang/Basic/SourceManager.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/Support/Casting.h"
 #include <algorithm>
 #include <cassert>
+#include <cmath>
 #include <cstdint>
 #include <string>
 #include <vector>
 
 using namespace clang::ast_matchers;
 using namespace clang::tidy::matchers;
 
 namespace clang {
 namespace tidy {
 namespace misc {
 
 namespace {
if (cast<UnaryOperator>(Left)->isIncrementDecrementOp())
       return false;
     return cast<UnaryOperator>(Left)->getOpcode() ==
            cast<UnaryOperator>(Right)->getOpcode();
   case Stmt::BinaryOperatorClass:
     return cast<BinaryOperator>(Left)->getOpcode() ==
            cast<BinaryOperator>(Right)->getOpcode();
   }
 }
 
 // For a given expression 'x', returns whether the ranges covered by the
 // relational operators are equivalent (i.e.  x <= 4 is equivalent to x < 5).
 static bool areEquivalentRanges(BinaryOperatorKind OpcodeLHS,
                                 const APSInt &ValueLHS,
                                 BinaryOperatorKind OpcodeRHS,
                                 const APSInt &ValueRHS) {
   assert(APSInt::compareValues(ValueLHS, ValueRHS) <= 0 &&
          "Values must be ordered");
   // Handle the case where constants are the same: x <= 4  <==>  x <= 4.
if (OpcodeLHS == BO_GT && OpcodeRHS == BO_LT &&
       incrementWithoutOverflow(ValueLHS, ValueLHS_plus1) &&
       APSInt::compareValues(ValueLHS_plus1, ValueRHS) == 0)
     return true;
 
   return false;
 }
 
 // Returns whether the ranges covered by the union of both relational
-// expressions covers the whole domain (i.e. x < 10  and  x > 0).
+// expressions cover the whole domain (i.e. x < 10  and  x > 0).
 static bool rangesFullyCoverDomain(BinaryOperatorKind OpcodeLHS,
                                    const APSInt &ValueLHS,
                                    BinaryOperatorKind OpcodeRHS,
                                    const APSInt &ValueRHS) {
   assert(APSInt::compareValues(ValueLHS, ValueRHS) <= 0 &&
          "Values must be ordered");
 
   // Handle cases where the constants are the same:  x < 5 || x >= 5.
if (!retrieveIntegerConstantExpr(Result, Id, Value, ConstExpr))
       return false;
   } else if (const auto *Cast = Result.Nodes.getNodeAs<CastExpr>(CastId)) {
     // Operand received with implicit comparator (cast).
     Opcode = BO_NE;
     OperandExpr = Cast;
     Value = APSInt(32, false);
   } else if (const auto *OverloadedOperatorExpr =
                  Result.Nodes.getNodeAs<CXXOperatorCallExpr>(OverloadId)) {
-    const auto *OverloadedFunctionDecl = dyn_cast_or_null<FunctionDecl>(OverloadedOperatorExpr->getCalleeDecl());
+    const auto *OverloadedFunctionDecl =
+        dyn_cast_or_null<FunctionDecl>(OverloadedOperatorExpr->getCalleeDecl());
     if (!OverloadedFunctionDecl)
       return false;
 
+    if (canOverloadedOperatorArgsBeModified(OverloadedFunctionDecl, false))
+      return false;
+
     if (!OverloadedOperatorExpr->getArg(1)->isIntegerConstantExpr(
             Value, *Result.Context))
       return false;
 
     Symbol = OverloadedOperatorExpr->getArg(0);
     OperandExpr = OverloadedOperatorExpr;
-    Opcode = BinaryOperator::getOverloadedOpcode(OverloadedOperatorExpr->getOperator());
+    Opcode = BinaryOperator::getOverloadedOpcode(
+        OverloadedOperatorExpr->getOperator());
 
     return BinaryOperator::isComparisonOp(Opcode);
   } else {
     return false;
   }
 
   if (!retrieveSymbolicExpr(Result, Id, Symbol))
     return false;
 
   if (Result.Nodes.getNodeAs<Expr>(SwapId))
     Opcode = BinaryOperator::reverseComparisonOp(Opcode);
   if (Result.Nodes.getNodeAs<Expr>(NegateId))
     Opcode = BinaryOperator::negateComparisonOp(Opcode);
   return true;
 }
 
 // Checks for expressions like (X == 4) && (Y != 9)
-static bool areSidesBinaryConstExpressions(const BinaryOperator *&BinOp, const ASTContext *AstCtx) {
+static bool areSidesBinaryConstExpressions(const BinaryOperator *&BinOp,
+                                           const ASTContext *AstCtx) {
   const auto *LhsBinOp = dyn_cast<BinaryOperator>(BinOp->getLHS());
   const auto *RhsBinOp = dyn_cast<BinaryOperator>(BinOp->getRHS());
 
   if (!LhsBinOp || !RhsBinOp)
     return false;
 
   if ((LhsBinOp->getLHS()->isIntegerConstantExpr(*AstCtx) ||
        LhsBinOp->getRHS()->isIntegerConstantExpr(*AstCtx)) &&
       (RhsBinOp->getLHS()->isIntegerConstantExpr(*AstCtx) ||
        RhsBinOp->getRHS()->isIntegerConstantExpr(*AstCtx)))
     return true;
   return false;
 }
 
 // Retrieves integer constant subexpressions from binary operator expressions
-// that have two equivalent sides
+// that have two equivalent sides.
 // E.g.: from (X == 5) && (X == 5) retrieves 5 and 5.
 static bool retrieveConstExprFromBothSides(const BinaryOperator *&BinOp,
                                            BinaryOperatorKind &MainOpcode,
                                            BinaryOperatorKind &SideOpcode,
                                            const Expr *&LhsConst,
                                            const Expr *&RhsConst,
                                            const ASTContext *AstCtx) {
   assert(areSidesBinaryConstExpressions(BinOp, AstCtx) &&
static bool areExprsFromDifferentMacros(const Expr *LhsExpr,
 
   if (!LhsLoc.isMacroID() || !RhsLoc.isMacroID())
     return false;
 
   const SourceManager &SM = AstCtx->getSourceManager();
   const LangOptions &LO = AstCtx->getLangOpts();
 
   return !(Lexer::getImmediateMacroName(LhsLoc, SM, LO) ==
            Lexer::getImmediateMacroName(RhsLoc, SM, LO));
 }
 
 static bool areExprsMacroAndNonMacro(const Expr *&LhsExpr,
                                      const Expr *&RhsExpr) {
   if (!LhsExpr || !RhsExpr)
     return false;
 
   SourceLocation LhsLoc = LhsExpr->getExprLoc();
Finder->addMatcher(
           parametersAreEquivalent(),
           // Filter noisy false positives.
           unless(isMacro()), unless(isInTemplateInstantiation()))
           .bind("call"),
       this);
 
   // Match ineffective bitwise operator expressions like: x & 0 or x |= ~0.
   Finder->addMatcher(
-      binaryOperator(anyOf(hasOperatorName("|"), hasOperatorName("&"),
-                           hasOperatorName("|="), hasOperatorName("&=")),
-                     hasEitherOperand(matchIntegerConstantExpr("ineffective-bitwise")),
-                     hasEitherOperand(matchSymbolicExpr("ineffective-bitwise")))
+      binaryOperator(
+          anyOf(hasOperatorName("|"), hasOperatorName("&"),
           .bind("ineffective-bitwise"),
       this);
 
+  // Match expressions like: !(1 | 2 | 3)
+  Finder->addMatcher(
+      implicitCastExpr(
+          hasImplicitDestinationType(isInteger()),
+          has(unaryOperator(
+                  hasOperatorName("!"),
+                  hasUnaryOperand(ignoringParenImpCasts(binaryOperator(
+                      anyOf(hasOperatorName("|"), hasOperatorName("&")),
+                      hasLHS(anyOf(binaryOperator(anyOf(hasOperatorName("|"),
+                                                        hasOperatorName("&"))),
+                                   integerLiteral())),
+                      hasRHS(integerLiteral())))))
+                  .bind("logical-bitwise-confusion"))),
+      this);
+
+  // Match expressions like: (X << 8) & 0xFF
+  Finder->addMatcher(
+      binaryOperator(hasOperatorName("&"),
+                     hasEitherOperand(ignoringParenImpCasts(binaryOperator(
+                         hasOperatorName("<<"),
+                         hasRHS(ignoringParenImpCasts(
+                             integerLiteral().bind("shift-const")))))),
+                     hasEitherOperand(ignoringParenImpCasts(
+                         integerLiteral().bind("and-const"))))
+          .bind("left-right-shift-confusion"),
+      this);
+
   // Match common expressions and apply more checks to find redundant
   // sub-expressions.
   //   a) Expr <op> K1 == K2
   //   b) Expr <op> K1 == Expr
   //   c) Expr <op> K1 == Expr <op> K2
   // see: 'checkArithmeticExpr' and 'checkBitwiseExpr'
   const auto BinOpCstLeft = matchBinOpIntegerConstantExpr("lhs");
   const auto BinOpCstRight = matchBinOpIntegerConstantExpr("rhs");
if (const auto *ComparisonOperator = Result.Nodes.getNodeAs<BinaryOperator>(
     }
   } else if (const auto *IneffectiveOperator =
                  Result.Nodes.getNodeAs<BinaryOperator>(
                      "ineffective-bitwise")) {
     APSInt Value;
     const Expr *Sym = nullptr, *ConstExpr = nullptr;
 
     if (!retrieveSymbolicExpr(Result, "ineffective-bitwise", Sym) ||
-        !retrieveIntegerConstantExpr(Result, "ineffective-bitwise", Value, ConstExpr))
+        !retrieveIntegerConstantExpr(Result, "ineffective-bitwise", Value,
+                                     ConstExpr))
       return;
 
-    if((Value != 0 && ~Value != 0) || Sym->getExprLoc().isMacroID())
-        return;
+    if ((Value != 0 && ~Value != 0) || Sym->getExprLoc().isMacroID())
 
     SourceLocation Loc = IneffectiveOperator->getOperatorLoc();
 
     BinaryOperatorKind Opcode = IneffectiveOperator->getOpcode();
     if (exprEvaluatesToZero(Opcode, Value)) {
       diag(Loc, "expression always evaluates to 0");
     } else if (exprEvaluatesToBitwiseNegatedZero(Opcode, Value)) {
       SourceRange ConstExprRange(ConstExpr->getLocStart(),
                                  ConstExpr->getLocEnd());
-      std::string ConstExprText = Lexer::getSourceText(
+      StringRef ConstExprText = Lexer::getSourceText(
           CharSourceRange::getTokenRange(ConstExprRange), *Result.SourceManager,
           Result.Context->getLangOpts());
 
-      diag(Loc,
-           "expression always evaluates to '" + ConstExprText + "'");
+      diag(Loc, "expression always evaluates to '%0'") << ConstExprText;
     } else if (exprEvaluatesToSymbolic(Opcode, Value)) {
       SourceRange SymExprRange(Sym->getLocStart(), Sym->getLocEnd());
 
-      std::string ExprText = Lexer::getSourceText(
+      StringRef ExprText = Lexer::getSourceText(
           CharSourceRange::getTokenRange(SymExprRange), *Result.SourceManager,
           Result.Context->getLangOpts());
 
-      diag(Loc, "expression always evaluates to '" + ExprText + "'");
+      diag(Loc, "expression always evaluates to '%0'") << ExprText;
     }
   }
 }
 
 void RedundantExpressionCheck::checkRelationalExpr(
     const MatchFinder::MatchResult &Result) {
   if (const auto *ComparisonOperator = Result.Nodes.getNodeAs<BinaryOperator>(
           "comparisons-of-symbol-and-const")) {
if (const auto *CondOp =
     if (areExprsFromDifferentMacros(TrueExpr, FalseExpr, Result.Context) ||
         areExprsMacroAndNonMacro(TrueExpr, FalseExpr))
       return;
     diag(CondOp->getColonLoc(),
          "'true' and 'false' expressions are equivalent");
   }
 
   if (const auto *Call = Result.Nodes.getNodeAs<CXXOperatorCallExpr>("call")) {
-    const auto *OverloadedFunctionDecl = dyn_cast_or_null<FunctionDecl>(Call->getCalleeDecl());
+    const auto *OverloadedFunctionDecl =
+        dyn_cast_or_null<FunctionDecl>(Call->getCalleeDecl());
     if (!OverloadedFunctionDecl)
       return;
 
     if (canOverloadedOperatorArgsBeModified(OverloadedFunctionDecl, true))
       return;
 
     diag(Call->getOperatorLoc(),
          "both sides of overloaded operator are equivalent");
   }
 
+  if (const auto *NegateOperator =
+          Result.Nodes.getNodeAs<UnaryOperator>("logical-bitwise-confusion")) {
+    SourceLocation OperatorLoc = NegateOperator->getOperatorLoc();
+
+    auto Diag =
+        diag(OperatorLoc,
+             "ineffective logical negation operator used; did you mean '~'?");
+    SourceLocation LogicalNotLocation = OperatorLoc.getLocWithOffset(1);
+
+    if (!LogicalNotLocation.isMacroID())
+      Diag << FixItHint::CreateReplacement(
+          CharSourceRange::getCharRange(OperatorLoc, LogicalNotLocation), "~");
+  }
+
+  if (const auto *BinaryAndExpr = Result.Nodes.getNodeAs<BinaryOperator>(
+          "left-right-shift-confusion")) {
+    const auto *ShiftingConst = Result.Nodes.getNodeAs<Expr>("shift-const");
+    assert(ShiftingConst && "Expr* 'ShiftingConst' is nullptr!");
+    APSInt ShiftingValue;
+
+    if (!ShiftingConst->isIntegerConstantExpr(ShiftingValue, *Result.Context))
+      return;
+
+    const auto *AndConst = Result.Nodes.getNodeAs<Expr>("and-const");
+    assert(AndConst && "Expr* 'AndCont' is nullptr!");
+    APSInt AndValue;
+    if (!AndConst->isIntegerConstantExpr(AndValue, *Result.Context))
+      return;
+
+    // If ShiftingConst is shifted left with more bits than the position of the
+    // leftmost 1 in the bit representation of AndValue, AndConstant is
+    // ineffective.
+    if (floor(log2(AndValue.getExtValue())) >= ShiftingValue)
+      return;
+
+    auto Diag = diag(BinaryAndExpr->getOperatorLoc(),
+                     "ineffective bitwise and operation");
+  }
+
   // Check for the following bound expressions:
   // - "binop-const-compare-to-sym",
   // - "binop-const-compare-to-binop-const",
   // Produced message:
   // -> "logical expression is always false/true"
   checkArithmeticExpr(Result);
 
   // Check for the following bound expression:
   // - "binop-const-compare-to-const",
+  // - "ineffective-bitwise"
   // Produced message:
   // -> "logical expression is always false/true"
+  // -> "expression always evaluates to ..."
   checkBitwiseExpr(Result);
 
   // Check for te following bound expression:
   // - "comparisons-of-symbol-and-const",
   // Produced messages:
   // -> "equivalent expression on both sides of logical operator",
   // -> "logical expression is always false/true"
   // -> "expression is redundant"
   checkRelationalExpr(Result);
 }
 
 } // namespace misc
 } // namespace tidy
 } // namespace clang

docs/ReleaseNotes.rst

- New `readability-static-accessed-through-instance
   replaces them with uses of the appropriate qualified-id.
 
 - Added `modernize-use-emplace.IgnoreImplicitConstructors
   <http://clang.llvm.org/extra/clang-tidy/checks/modernize-use-emplace.html#cmdoption-arg-IgnoreImplicitConstructors>`_
   option.
 
 - Added alias `hicpp-braces-around-statements <http://clang.llvm.org/extra/clang-tidy/checks/hicpp-braces-around-statements.html>`_ 
 
+- Added new functionality to `misc-redundant-expression
+  http://clang.llvm.org/extra/clang-tidy/checks/misc-redundant-expression.html`_ check
+
+  Finds redundant binary operator expressions where the operators are overloaded,
+  and ones that contain the same macros twice.
+  Also checks for assignment expressions that do not change the value of the
+  assigned variable, and expressions that always evaluate to the same value
+  because of possible operator confusion.
+
 Improvements to include-fixer
 -----------------------------
 
 The improvements are...
 
 Improvements to modularize
 --------------------------
 
 The improvements are...

test/clang-tidy/misc-redundant-expression.cpp

bool TestOverloadedOperator(MyStruct& S) {
   if (S == S) return true;
   // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of overloaded operator are equivalent
   if (S < S) return true;
   // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of overloaded operator are equivalent
   if (S != S) return true;
   // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of overloaded operator are equivalent
   if (S >= S) return true;
   // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: both sides of overloaded operator are equivalent
+
+  return true;
 }
 
 #define LT(x, y) (void)((x) < (y))
 #define COND(x, y, z) ((x)?(y):(z))
 #define EQUALS(x, y) (x) == (y)
 
 int TestMacro(int X, int Y) {
   LT(0, 0);
int TestWithMinMaxInt(int X) {
   if (X + 0x80000000 == X - 0x80000000) return 1;
   // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: logical expression is always true
 
   if (X > 0x7FFFFFFF || X < ((-0x7FFFFFFF)-1)) return 1;
   if (X <= 0x7FFFFFFF && X >= ((-0x7FFFFFFF)-1)) return 1;
 
   return 0;
 }
+
+#define FLAG1 1
+#define FLAG2 2
+#define FLAG3 4
+#define FLAGS (FLAG1 | FLAG2 | FLAG3)
+#define NOTFLAGS !(FLAG1 | FLAG2 | FLAG3)
+int operatorConfusion(int X, int Y, long Z)
+{
+  // Ineffective & expressions.
+  Y = (Y << 8) & 0xff;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: ineffective bitwise and operation
+  Y = (Y << 12) & 0xfff;
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: ineffective bitwise and
+  Y = (Y << 12) & 0xff;
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: ineffective bitwise and
+  Y = (Y << 8) & 0x77;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: ineffective bitwise and
+  Y = (Y << 5) & 0x11;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: ineffective bitwise and
+
+  // Tests for unmatched types
+  Z = (Z << 8) & 0xff;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: ineffective bitwise and operation
+  Y = (Y << 12) & 0xfffL;
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: ineffective bitwise and
+  Z = (Y << 12) & 0xffLL;
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: ineffective bitwise and
+  Y = (Z << 8L) & 0x77L;
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: ineffective bitwise and
+
+  // Effective expressions. Do not check.
+  Y = (Y << 4) & 0x15;
+  Y = (Y << 3) & 0x250;
+  Y = (Y << 9) & 0xF33;
+
+  int K = !(1 | 2 | 4);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: ineffective logical negation operator used; did you mean '~'?
+  // CHECK-FIXES: {{^}}  int K = ~(1 | 2 | 4);{{$}}
+  K = !(FLAG1 & FLAG2 & FLAG3);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: ineffective logical negation operator
+  // CHECK-FIXES: {{^}}  K = ~(FLAG1 & FLAG2 & FLAG3);{{$}}
+  K = !(3 | 4);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: ineffective logical negation operator
+  // CHECK-FIXES: {{^}}  K = ~(3 | 4);{{$}}
+  int NotFlags = !FLAGS;
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: ineffective logical negation operator
+  // CHECK-FIXES: {{^}}  int NotFlags = ~FLAGS;{{$}}
+  NotFlags = NOTFLAGS;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: ineffective logical negation operator
+  return !(1 | 2 | 4);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: ineffective logical negation operator
+  // CHECK-FIXES: {{^}}  return ~(1 | 2 | 4);{{$}}
+}
+#undef FLAG1
+#undef FLAG2
+#undef FLAG3