https://github.com/schenker updated
https://github.com/llvm/llvm-project/pull/71974
>From 1f6e70ef97ba1bf90c829c212c6c1e09be4961f4 Mon Sep 17 00:00:00 2001
From: Thomas Schenker <thomas.schen...@protonmail.com>
Date: Fri, 10 Nov 2023 18:58:26 +0100
Subject: [PATCH 1/5] [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);
```
---
.../clang-tidy/bugprone/AssertSideEffectCheck.cpp | 4 ++++
.../checkers/bugprone/assert-side-effect.cpp | 13 ++++++++++++-
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
index 07a987359d4d8d7..599f5ac70e7a229 100644
--- a/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
@@ -41,6 +41,10 @@ AST_MATCHER_P2(Expr, hasSideEffect, bool, CheckFunctionCalls,
}
if (const auto *OpCallExpr = dyn_cast<CXXOperatorCallExpr>(E)) {
+ if (const auto *FuncDecl = OpCallExpr->getDirectCallee())
+ if (const auto *MethodDecl = dyn_cast<CXXMethodDecl>(FuncDecl))
+ return !MethodDecl->isConst();
+
OverloadedOperatorKind OpKind = OpCallExpr->getOperator();
return OpKind == OO_Equal || OpKind == OO_PlusEqual ||
OpKind == OO_MinusEqual || OpKind == OO_StarEqual ||
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..49d3d456deb9d35 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
@@ -11,7 +11,7 @@ class MyClass {
MyClass &operator=(const MyClass &rhs) { return *this; }
- int operator-() { return 1; }
+ int operator-() const { return 1; }
operator bool() const { return true; }
@@ -84,5 +84,16 @@ int main() {
msvc_assert(mc2 = mc);
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: side effect in msvc_assert()
condition discarded in release builds
+ struct {
+ int operator<<(int i) const { return i; }
+ } constOp;
+ assert(constOp << 1);
+
+ struct {
+ int operator<<(int i) { return i; }
+ } nonConstOp;
+ assert(nonConstOp << 1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: side effect in assert()
condition discarded in release builds
+
return 0;
}
>From 25dff1d9289e154fa908412584b4b04b862182ea Mon Sep 17 00:00:00 2001
From: Thomas Schenker <thomas.schen...@protonmail.com>
Date: Fri, 10 Nov 2023 21:07:39 +0100
Subject: [PATCH 2/5] use dyn_cast_or_null and add release note
---
.../clang-tidy/bugprone/AssertSideEffectCheck.cpp | 6 +++---
clang-tools-extra/docs/ReleaseNotes.rst | 4 ++++
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
index 599f5ac70e7a229..a6c01704b3d909f 100644
--- a/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
@@ -41,9 +41,9 @@ AST_MATCHER_P2(Expr, hasSideEffect, bool, CheckFunctionCalls,
}
if (const auto *OpCallExpr = dyn_cast<CXXOperatorCallExpr>(E)) {
- if (const auto *FuncDecl = OpCallExpr->getDirectCallee())
- if (const auto *MethodDecl = dyn_cast<CXXMethodDecl>(FuncDecl))
- return !MethodDecl->isConst();
+ if (const auto *MethodDecl =
+ dyn_cast_or_null<CXXMethodDecl>(OpCallExpr->getDirectCallee()))
+ return !MethodDecl->isConst();
OverloadedOperatorKind OpKind = OpCallExpr->getOperator();
return OpKind == OO_Equal || OpKind == OO_PlusEqual ||
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index 353c6fe20269274..d417036f19d565e 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -212,6 +212,10 @@ 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 operator methods in assertions.
+
- 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
>From f8e93b572c0198f9e95e1aa90b788a0a7f828b42 Mon Sep 17 00:00:00 2001
From: Thomas Schenker <thomas.schen...@protonmail.com>
Date: Fri, 10 Nov 2023 21:45:52 +0100
Subject: [PATCH 3/5] test const overload
---
.../checkers/bugprone/assert-side-effect.cpp | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
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 49d3d456deb9d35..c39cb422ebc68cc 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,15 +84,16 @@ int main() {
msvc_assert(mc2 = mc);
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: side effect in msvc_assert()
condition discarded in release builds
- struct {
+ struct ConstOverload {
int operator<<(int i) const { return i; }
- } constOp;
- assert(constOp << 1);
-
- struct {
int operator<<(int i) { return i; }
- } nonConstOp;
- assert(nonConstOp << 1);
+ };
+
+ const ConstOverload const_instance;
+ assert(const_instance << 1);
+
+ ConstOverload mutable_instance;
+ assert(mutable_instance << 1);
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: side effect in assert()
condition discarded in release builds
return 0;
>From 86d89486e56868999705247cd5864c3cc729bd3e Mon Sep 17 00:00:00 2001
From: Thomas Schenker <thomas.schen...@protonmail.com>
Date: Mon, 13 Nov 2023 21:54:03 +0100
Subject: [PATCH 4/5] always consider const operator calls side-effect free,
add << and >> operators to the list of operators potentially having side
effects
---
.../bugprone/AssertSideEffectCheck.cpp | 4 +++-
clang-tools-extra/docs/ReleaseNotes.rst | 3 ++-
.../checkers/bugprone/assert-side-effect.cpp | 20 ++++++++++++++-----
3 files changed, 20 insertions(+), 7 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
index a6c01704b3d909f..43bedd4f73ef447 100644
--- a/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
@@ -43,7 +43,8 @@ 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()))
- return !MethodDecl->isConst();
+ if (MethodDecl->isConst())
+ return false;
OverloadedOperatorKind OpKind = OpCallExpr->getOperator();
return OpKind == OO_Equal || OpKind == OO_PlusEqual ||
@@ -51,6 +52,7 @@ AST_MATCHER_P2(Expr, hasSideEffect, bool, CheckFunctionCalls,
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 d417036f19d565e..526fec43e98a6d4 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -214,7 +214,8 @@ Changes in existing checks
- Improved :doc:`bugprone-assert-side-effect
<clang-tidy/checks/bugprone/assert-side-effect>` check to report usage of
- non-const operator methods in assertions.
+ 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
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 c39cb422ebc68cc..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
@@ -11,7 +11,7 @@ class MyClass {
MyClass &operator=(const MyClass &rhs) { return *this; }
- int operator-() const { return 1; }
+ int operator-() { return 1; }
operator bool() const { return true; }
@@ -84,17 +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 ConstOverload {
+ 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 ConstOverload const_instance;
+ const OperatorTest const_instance;
assert(const_instance << 1);
+ assert(const_instance += 1);
- ConstOverload mutable_instance;
- assert(mutable_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;
}
>From 12b5edfda8da19eec7f2427c20b7913943123b76 Mon Sep 17 00:00:00 2001
From: schenker <schen...@users.noreply.github.com>
Date: Wed, 15 Nov 2023 22:24:48 +0100
Subject: [PATCH 5/5] Update clang-tools-extra/docs/ReleaseNotes.rst
Co-authored-by: Piotr Zegar <m...@piotrzegar.pl>
---
clang-tools-extra/docs/ReleaseNotes.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index 526fec43e98a6d4..23111be4371e2e1 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -214,7 +214,7 @@ Changes in existing checks
- 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
+ non-const ``<<`` and ``>>`` operators in assertions and fixed some
false-positives
with const operators.
- Improved :doc:`bugprone-dangling-handle
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
