[clang-tools-extra] [clang-tidy] bugprone-assert-side-effect can detect side effect from non-const reference parameters (PR #84095)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-tools-extra

Author: Congcong Cai (HerrCai0907)

<details>
<summary>Changes</summary>

Fixes: #<!-- -->84092

---
Full diff: https://github.com/llvm/llvm-project/pull/84095.diff


3 Files Affected:

- (modified) clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp (+17-5) 
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+4) 
- (modified) clang-tools-extra/test/clang-tidy/checkers/bugprone/assert-side-effect.cpp (+24) 


``````````diff
diff --git a/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
index 43bedd4f73ef44..2111047b9dff7a 100644
--- a/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
@@ -13,6 +13,7 @@
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Lex/Lexer.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
@@ -60,16 +61,27 @@ AST_MATCHER_P2(Expr, hasSideEffect, bool, CheckFunctionCalls,
   }
 
   if (const auto *CExpr = dyn_cast<CallExpr>(E)) {
-    bool Result = CheckFunctionCalls;
+    if (!CheckFunctionCalls)
+      return false;
     if (const auto *FuncDecl = CExpr->getDirectCallee()) {
       if (FuncDecl->getDeclName().isIdentifier() &&
           IgnoredFunctionsMatcher.matches(*FuncDecl, Finder,
                                           Builder)) // exceptions come here
-        Result = false;
-      else if (const auto *MethodDecl = dyn_cast<CXXMethodDecl>(FuncDecl))
-        Result &= !MethodDecl->isConst();
+        return false;
+      for (size_t I = 0; I < FuncDecl->getNumParams(); I++) {
+        const ParmVarDecl *P = FuncDecl->getParamDecl(I);
+        const Expr *ArgExpr =
+            I < CExpr->getNumArgs() ? CExpr->getArg(I) : nullptr;
+        QualType PT = P->getType().getCanonicalType();
+        if (PT->isReferenceType() &&
+            !PT.getNonReferenceType().isConstQualified() && ArgExpr &&
+            !ArgExpr->isXValue())
+          return true;
+      }
+      if (const auto *MethodDecl = dyn_cast<CXXMethodDecl>(FuncDecl))
+        return !MethodDecl->isConst();
     }
-    return Result;
+    return true;
   }
 
   return isa<CXXNewExpr>(E) || isa<CXXDeleteExpr>(E) || isa<CXXThrowExpr>(E);
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 0d2467210fc664..0d4d5ddb5c8763 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -122,6 +122,10 @@ New check aliases
 Changes in existing checks
 ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+- Improved :doc:`bugprone-assert-side-effect
+  <clang-tidy/checks/bugprone/assert-side-effect>` check by detection side
+  effect from method call with non-const reference parameters.
+
 - Improved :doc:`bugprone-non-zero-enum-to-bool-conversion
   <clang-tidy/checks/bugprone/non-zero-enum-to-bool-conversion>` check by
   eliminating false positives resulting from direct usage of bitwise operators
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 c11638aa823aae..5cdc1afb3d9099 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
@@ -108,3 +108,27 @@ int main() {
 
   return 0;
 }
+
+namespace parameter_anaylysis {
+
+struct S {
+  bool value(int) const;
+  bool leftValueRef(int &) const;
+  bool constRef(int const &) const;
+  bool rightValueRef(int &&) const;
+};
+
+void foo() {
+  S s{};
+  int i = 0;
+  assert(s.value(0));
+  assert(s.value(i));
+  assert(s.leftValueRef(i));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: side effect in assert() condition discarded in release builds
+  assert(s.constRef(0));
+  assert(s.constRef(i));
+  assert(s.rightValueRef(0));
+  assert(s.rightValueRef(static_cast<int &&>(i)));
+}
+
+} // namespace parameter_anaylysis

``````````

</details>


https://github.com/llvm/llvm-project/pull/84095