[clang-tidy] Never consider assignments as equivalent in `misc-redundant-expression` check

[fwolff]

Summary

Attempts to fix #35853. This issue is about side-effectful function calls, but since almost every function call potentially causes side effects and would therefore prevent any misc-redundant-expression warnings, I have focused on assignments instead, which would also fix the example in #35853.

We already have special treatment of unary increment/decrement here, even though this can also lead to false negatives. Replacing i++ with i = i + 1 currently changes the behavior of the misc-redundant-expression check, whereas the change proposed here prevents any warnings about redundant expressions if assignments are involved. This can cause false negatives, though (whereas the current implementation causes false positives), so let me know whether you think this is a sensible change to make.

[aaron.ballman]

LGTM; can you add a release note when you land?

[njames93]

FWIW, False negatives are more tolerable than false positives in clang tidy.

clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp

static bool areEquivalentExpr(const Expr *Left, const Expr *Right) {
   case Stmt::ArraySubscriptExprClass:
     return true;
   case Stmt::UnaryOperatorClass:
     if (cast<UnaryOperator>(Left)->isIncrementDecrementOp())
       return false;
     return cast<UnaryOperator>(Left)->getOpcode() ==
            cast<UnaryOperator>(Right)->getOpcode();
   case Stmt::BinaryOperatorClass:
+    if (cast<BinaryOperator>(Left)->isAssignmentOp())
+      return false;
     return cast<BinaryOperator>(Left)->getOpcode() ==
            cast<BinaryOperator>(Right)->getOpcode();
   case Stmt::UnaryExprOrTypeTraitExprClass:
     const auto *LeftUnaryExpr =
         cast<UnaryExprOrTypeTraitExpr>(Left);
     const auto *RightUnaryExpr =
         cast<UnaryExprOrTypeTraitExpr>(Right);
     if (LeftUnaryExpr->isArgumentType() && RightUnaryExpr->isArgumentType())

clang-tools-extra/docs/ReleaseNotes.rst

 Changes in existing checks
 ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 - Improved :doc:`performance-inefficient-vector-operation 
   <clang-tidy/checks/performance-inefficient-vector-operation>` to work when
   the vector is a member of a structure.
 
 - Fixed a false positive in :doc:`readability-non-const-parameter
-  <clang-tidy/checks/readability-non-const-parameter>` when the parameter is referenced by an lvalue
+  <clang-tidy/checks/readability-non-const-parameter>` when the parameter is referenced by an lvalue.
 
 - Fixed a crash in :doc:`readability-const-return-type
   <clang-tidy/checks/readability-const-return-type>` when a pure virtual function
   overrided has a const return type. Removed the fix for a virtual function.
 
 - Fixed a false positive in :doc:`misc-redundant-expression <clang-tidy/checks/misc-redundant-expression>`
   involving overloaded comparison operators.
 
 - Fixed a crash in :doc:`bugprone-sizeof-expression <clang-tidy/checks/bugprone-sizeof-expression>` when
   `sizeof(...)` is compared agains a `__int128_t`.
   
 - Improved :doc:`cppcoreguidelines-prefer-member-initializer
   <clang-tidy/checks/cppcoreguidelines-prefer-member-initializer>` check.
 
   Fixed an issue when there was already an initializer in the constructor and
   the check would try to create another initializer for the same member.
 
+- Fixed a false positive in :doc:`misc-redundant-expression <clang-tidy/checks/misc-redundant-expression>`
+  involving assignments in conditions. This fixes `Issue 35853 <https://github.com/llvm/llvm-project/issues/35853>`_.
+
 Removed checks
 ^^^^^^^^^^^^^^
 
 Improvements to include-fixer
 -----------------------------
 
 The improvements are...
 

clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp

struct Bar2 {
   // have side-effects on its operands. For other constructs with the same
   // property (eg, the `S2` cases above) we suppress this diagnostic. This
   // started failing when Clang started properly modeling the fold-expression as
   // containing an unresolved operator name.
   // FIXME-MESSAGES: :[[@LINE-1]]:47: warning: both sides of operator are equivalent [misc-redundant-expression]
 };
 
 } // namespace no_crash
+
+int TestAssignSideEffect(int i) {
+  int k = i;
+
+  if ((k = k + 1) != 1 || (k = k + 1) != 2)
+    return 0;
+
+  if ((k = foo(0)) != 1 || (k = foo(0)) != 2)
+    return 1;
+
+  return 2;
+}