[clang-tools-extra] [clang-tidy] bugprone-assert-side-effect non-const operator methods (PR #71974)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Nov 16 00:15:51 PST 2023
https://github.com/schenker updated https://github.com/llvm/llvm-project/pull/71974
>From d899ad40afd6a84210313e6e5f94bb1d678a4778 Mon Sep 17 00:00:00 2001
From: Thomas Schenker <thomas.schenker at protonmail.com>
Date: Fri, 10 Nov 2023 18:58:26 +0100
Subject: [PATCH] [clang-tidy] bugprone-assert-side-effect non-const operator
methods
With this PR, `bugprone-assert-side-effect` assumes that operator methods
that are not marked as `const` have side effects. This matches the
existing rule for non-operator methods.
E.g. the following snippet is now reported and was previously not:
```
std::stringstream ss;
assert(ss << 1);
```
---
.../bugprone/AssertSideEffectCheck.cpp | 6 +++++
clang-tools-extra/docs/ReleaseNotes.rst | 5 +++++
.../checkers/bugprone/assert-side-effect.cpp | 22 +++++++++++++++++++
3 files changed, 33 insertions(+)
diff --git a/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
index 07a987359d4d8d7..43bedd4f73ef447 100644
--- a/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
@@ -41,12 +41,18 @@ AST_MATCHER_P2(Expr, hasSideEffect, bool, CheckFunctionCalls,
}
if (const auto *OpCallExpr = dyn_cast<CXXOperatorCallExpr>(E)) {
+ if (const auto *MethodDecl =
+ dyn_cast_or_null<CXXMethodDecl>(OpCallExpr->getDirectCallee()))
+ if (MethodDecl->isConst())
+ return false;
+
OverloadedOperatorKind OpKind = OpCallExpr->getOperator();
return OpKind == OO_Equal || OpKind == OO_PlusEqual ||
OpKind == OO_MinusEqual || OpKind == OO_StarEqual ||
OpKind == OO_SlashEqual || OpKind == OO_AmpEqual ||
OpKind == OO_PipeEqual || OpKind == OO_CaretEqual ||
OpKind == OO_LessLessEqual || OpKind == OO_GreaterGreaterEqual ||
+ OpKind == OO_LessLess || OpKind == OO_GreaterGreater ||
OpKind == OO_PlusPlus || OpKind == OO_MinusMinus ||
OpKind == OO_PercentEqual || OpKind == OO_New ||
OpKind == OO_Delete || OpKind == OO_Array_New ||
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 353c6fe20269274..23111be4371e2e1 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -212,6 +212,11 @@ Changes in existing checks
<clang-tidy/checks/abseil/string-find-startswith>` check to also consider
``std::basic_string_view`` in addition to ``std::basic_string`` by default.
+- Improved :doc:`bugprone-assert-side-effect
+ <clang-tidy/checks/bugprone/assert-side-effect>` check to report usage of
+ non-const ``<<`` and ``>>`` operators in assertions and fixed some false-positives
+ with const operators.
+
- Improved :doc:`bugprone-dangling-handle
<clang-tidy/checks/bugprone/dangling-handle>` check to support functional
casting during type conversions at variable initialization, now with improved
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 6c41e1e320adeac..c11638aa823aaeb 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
@@ -84,5 +84,27 @@ int main() {
msvc_assert(mc2 = mc);
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: side effect in msvc_assert() condition discarded in release builds
+ struct OperatorTest {
+ int operator<<(int i) const { return i; }
+ int operator<<(int i) { return i; }
+ int operator+=(int i) const { return i; }
+ int operator+=(int i) { return i; }
+ };
+
+ const OperatorTest const_instance;
+ assert(const_instance << 1);
+ assert(const_instance += 1);
+
+ OperatorTest non_const_instance;
+ assert(static_cast<const OperatorTest>(non_const_instance) << 1);
+ assert(static_cast<const OperatorTest>(non_const_instance) += 1);
+ assert(non_const_instance << 1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: side effect in assert() condition discarded in release builds
+ assert(non_const_instance += 1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: side effect in assert() condition discarded in release builds
+
+ assert(5<<1);
+ assert(5>>1);
+
return 0;
}
More information about the cfe-commits
mailing list