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 <congcongcai0...@163.com> Date: Wed, 6 Mar 2024 06:55:30 +0800 Subject: [PATCH 1/3] [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 <congcongcai0...@163.com> Date: Wed, 6 Mar 2024 07:44:15 +0800 Subject: [PATCH 2/3] [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 <congcongcai0...@163.com> Date: Wed, 6 Mar 2024 07:46:28 +0800 Subject: [PATCH 3/3] 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" _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits