[clang-tools-extra] [clang-tidy] bugprone-assert-side-effect can detect side effect from non-const reference parameters (PR #84095)
Congcong Cai via cfe-commits
cfe-commits at lists.llvm.org
Wed Mar 6 17:38:59 PST 2024
https://github.com/HerrCai0907 updated https://github.com/llvm/llvm-project/pull/84095
>From 160add4d95f34608a3d423482ad50fb45f6db522 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Wed, 6 Mar 2024 06:55:30 +0800
Subject: [PATCH 1/4] [clang-tidy][NFC]refactor AssertSideEffectCheck logic
---
.../clang-tidy/bugprone/AssertSideEffectCheck.cpp | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
index 43bedd4f73ef44..25ca42bfd1bede 100644
--- a/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
@@ -60,16 +60,17 @@ 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;
+ 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);
>From ce40ee88c708c8dcce8e5b2b9a1d000465e80c24 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Wed, 6 Mar 2024 07:44:15 +0800
Subject: [PATCH 2/4] [clang-tidy] bugprone-assert-side-effect can detect side
effect from non-const reference parameters
Fixes: #84092
---
.../bugprone/AssertSideEffectCheck.cpp | 11 +++++++++
clang-tools-extra/docs/ReleaseNotes.rst | 4 ++++
.../checkers/bugprone/assert-side-effect.cpp | 24 +++++++++++++++++++
3 files changed, 39 insertions(+)
diff --git a/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
index 25ca42bfd1bede..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"
@@ -67,6 +68,16 @@ AST_MATCHER_P2(Expr, hasSideEffect, bool, CheckFunctionCalls,
IgnoredFunctionsMatcher.matches(*FuncDecl, Finder,
Builder)) // exceptions come here
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();
}
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
>From dbc7d6b1bf08056eb964be64459a4324545cf3f0 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Wed, 6 Mar 2024 07:46:28 +0800
Subject: [PATCH 3/4] clean header
---
clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp | 1 -
1 file changed, 1 deletion(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
index 2111047b9dff7a..ee94db03234d45 100644
--- a/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
@@ -13,7 +13,6 @@
#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"
>From 4dcf303290b616b264e9e3f8ee1f410a31c52b67 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Thu, 7 Mar 2024 09:38:44 +0800
Subject: [PATCH 4/4] fix
---
.../clang-tidy/bugprone/AssertSideEffectCheck.cpp | 7 +++----
clang-tools-extra/docs/ReleaseNotes.rst | 4 ++--
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
index ee94db03234d45..c650aae4fa03c0 100644
--- a/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
@@ -71,10 +71,9 @@ AST_MATCHER_P2(Expr, hasSideEffect, bool, CheckFunctionCalls,
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())
+ const QualType PT = P->getType().getCanonicalType();
+ if (ArgExpr && !ArgExpr->isXValue() && PT->isReferenceType() &&
+ !PT.getNonReferenceType().isConstQualified())
return true;
}
if (const auto *MethodDecl = dyn_cast<CXXMethodDecl>(FuncDecl))
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 0d4d5ddb5c8763..0f8a35aed380cc 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -123,8 +123,8 @@ 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.
+ <clang-tidy/checks/bugprone/assert-side-effect>` check by detecting side
+ effect from calling a method 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
More information about the cfe-commits
mailing list