diff --git a/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp index 43bedd4f73ef4..c650aae4fa03c 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(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(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(FuncDecl)) + return !MethodDecl->isConst(); } - return Result; + return true; } return isa(E) || isa(E) || isa(E); diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 1b839a35c3ed6..d98c4ff9a7504 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 + ` check by detecting side + effect from calling a method with non-const reference parameters. + - Improved :doc:`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 c11638aa823aa..5cdc1afb3d909 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(i))); +} + +} // namespace parameter_anaylysis