Author: Congcong Cai Date: 2024-03-07T09:39:29+08:00 New Revision: 84842f4b3ba175d744c03d40baa8e371522615e8
URL: https://github.com/llvm/llvm-project/commit/84842f4b3ba175d744c03d40baa8e371522615e8 DIFF: https://github.com/llvm/llvm-project/commit/84842f4b3ba175d744c03d40baa8e371522615e8.diff LOG: [clang-tidy] bugprone-assert-side-effect can detect side effect from non-const reference parameters (#84095) Fixes: #84092 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 43bedd4f73ef44..c650aae4fa03c0 100644 --- a/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp @@ -60,16 +60,26 @@ 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; + 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)) + 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 1b839a35c3ed65..d98c4ff9a75044 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -128,6 +128,10 @@ New check aliases Changes in existing checks ^^^^^^^^^^^^^^^^^^^^^^^^^^ +- Improved :doc:`bugprone-assert-side-effect + <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 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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
