[clang-tools-extra] c95d581 - [clang-tidy] bugprone-assert-side-effect non-const operator methods (#71974)

[via cfe-commits]

Author: schenker
Date: 2023-11-16T09:53:40+01:00
New Revision: c95d5813b823bbb61cd7d676b959e64754098f49

URL: https://github.com/llvm/llvm-project/commit/c95d5813b823bbb61cd7d676b959e64754098f49
DIFF: https://github.com/llvm/llvm-project/commit/c95d5813b823bbb61cd7d676b959e64754098f49.diff

LOG: [clang-tidy] bugprone-assert-side-effect non-const operator methods (#71974)

With this PR, `bugprone-assert-side-effect` reports non-const calls to
`<<` and `>>` operators and does no longer consider any const operator
calls to have side effects.

E.g. the following snippet is now reported and was previously not:
```
std::stringstream ss;
assert(ss << 1);
```

The following snippet was previously a false-positive and is not
reported anymore:
```
struct {
  int operator+=(int i) const { return i; }
} t;
assert(t += 1);
```

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
    clang-tools-extra/docs/ReleaseNotes.rst
    clang-tools-extra/test/clang-tidy/checkers/bugprone/assert-side-effect.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
index 07a987359d4d8d7..43bedd4f73ef447 100644
--- a/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
@@ -41,12 +41,18 @@ AST_MATCHER_P2(Expr, hasSideEffect, bool, CheckFunctionCalls,
   }
 
   if (const auto *OpCallExpr = dyn_cast<CXXOperatorCallExpr>(E)) {
+    if (const auto *MethodDecl =
+            dyn_cast_or_null<CXXMethodDecl>(OpCallExpr->getDirectCallee()))
+      if (MethodDecl->isConst())
+        return false;
+
     OverloadedOperatorKind OpKind = OpCallExpr->getOperator();
     return OpKind == OO_Equal || OpKind == OO_PlusEqual ||
            OpKind == OO_MinusEqual || OpKind == OO_StarEqual ||
            OpKind == OO_SlashEqual || OpKind == OO_AmpEqual ||
            OpKind == OO_PipeEqual || OpKind == OO_CaretEqual ||
            OpKind == OO_LessLessEqual || OpKind == OO_GreaterGreaterEqual ||
+           OpKind == OO_LessLess || OpKind == OO_GreaterGreater ||
            OpKind == OO_PlusPlus || OpKind == OO_MinusMinus ||
            OpKind == OO_PercentEqual || OpKind == OO_New ||
            OpKind == OO_Delete || OpKind == OO_Array_New ||

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 353c6fe20269274..23111be4371e2e1 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -212,6 +212,11 @@ Changes in existing checks
   <clang-tidy/checks/abseil/string-find-startswith>` check to also consider
   ``std::basic_string_view`` in addition to ``std::basic_string`` by default.
 
+- Improved :doc:`bugprone-assert-side-effect
+  <clang-tidy/checks/bugprone/assert-side-effect>` check to report usage of
+  non-const ``<<`` and ``>>`` operators in assertions and fixed some false-positives
+  with const operators.
+
 - Improved :doc:`bugprone-dangling-handle
   <clang-tidy/checks/bugprone/dangling-handle>` check to support functional
   casting during type conversions at variable initialization, now with improved

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/assert-side-effect.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/assert-side-effect.cpp
index 6c41e1e320adeac..c11638aa823aaeb 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/assert-side-effect.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/assert-side-effect.cpp
@@ -84,5 +84,27 @@ int main() {
   msvc_assert(mc2 = mc);
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: side effect in msvc_assert() condition discarded in release builds
 
+  struct OperatorTest {
+    int operator<<(int i) const { return i; }
+    int operator<<(int i) { return i; }
+    int operator+=(int i) const { return i; }
+    int operator+=(int i) { return i; }
+  };
+
+  const OperatorTest const_instance;
+  assert(const_instance << 1);
+  assert(const_instance += 1);
+
+  OperatorTest non_const_instance;
+  assert(static_cast<const OperatorTest>(non_const_instance) << 1);
+  assert(static_cast<const OperatorTest>(non_const_instance) += 1);
+  assert(non_const_instance << 1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: side effect in assert() condition discarded in release builds
+  assert(non_const_instance += 1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: side effect in assert() condition discarded in release builds
+
+  assert(5<<1);
+  assert(5>>1);
+
   return 0;
 }