https://github.com/PiotrZSL closed
https://github.com/llvm/llvm-project/pull/71974
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
schenker wrote:
Thanks for pointing out, I squashed and changed the settings.
https://github.com/llvm/llvm-project/pull/71974
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
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
Date: Fri, 10 Nov 2023 18:58:26 +0100
Subject: [PATCH] [clang-tidy] bugprone-assert-side-effect non-const operator
https://github.com/PiotrZSL approved this pull request.
Code is fine and can be merged, unless you want it merged as
"schen...@users.noreply.github.com" you may need to disable email privacy
options or try to squash all commits in one.
https://github.com/llvm/llvm-project/pull/71974
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
Date: Fri, 10 Nov 2023 18:58:26 +0100
Subject: [PATCH 1/5] [clang-tidy] bugprone-assert-side-effect non-const
https://github.com/schenker updated
https://github.com/llvm/llvm-project/pull/71974
>From dabfdee1a982000605e4b33930ba433c63d1684f Mon Sep 17 00:00:00 2001
From: Thomas Schenker
Date: Fri, 10 Nov 2023 18:58:26 +0100
Subject: [PATCH 1/5] [clang-tidy] bugprone-assert-side-effect non-const
https://github.com/PiotrZSL approved this pull request.
LGTM, rebase code before merging
https://github.com/llvm/llvm-project/pull/71974
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
@@ -208,6 +208,11 @@ New check aliases
Changes in existing checks
^^
+- Improved :doc:`bugprone-assert-side-effect
+ ` check to report usage of
+ non-const `<<` and `>>` operators in assertions and fixed some
false-positives
https://github.com/schenker updated
https://github.com/llvm/llvm-project/pull/71974
>From dabfdee1a982000605e4b33930ba433c63d1684f Mon Sep 17 00:00:00 2001
From: Thomas Schenker
Date: Fri, 10 Nov 2023 18:58:26 +0100
Subject: [PATCH 1/4] [clang-tidy] bugprone-assert-side-effect non-const
schenker wrote:
@PiotrZSL I changed some things, please take another look.
All const operator calls are now considered to be side-effect free. This also
fixes some false-positives, e.g.
```
struct t {
int operator+=(int i) const { return i; }
} t;
assert(t += 1);
```
is no longer reported.
https://github.com/schenker updated
https://github.com/llvm/llvm-project/pull/71974
>From dabfdee1a982000605e4b33930ba433c63d1684f Mon Sep 17 00:00:00 2001
From: Thomas Schenker
Date: Fri, 10 Nov 2023 18:58:26 +0100
Subject: [PATCH 1/4] [clang-tidy] bugprone-assert-side-effect non-const
https://github.com/schenker edited
https://github.com/llvm/llvm-project/pull/71974
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/schenker edited
https://github.com/llvm/llvm-project/pull/71974
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/schenker edited
https://github.com/llvm/llvm-project/pull/71974
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/schenker edited
https://github.com/llvm/llvm-project/pull/71974
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -41,6 +41,10 @@ AST_MATCHER_P2(Expr, hasSideEffect, bool, CheckFunctionCalls,
}
if (const auto *OpCallExpr = dyn_cast(E)) {
+if (const auto *FuncDecl = OpCallExpr->getDirectCallee())
+ if (const auto *MethodDecl = dyn_cast(FuncDecl))
+return
https://github.com/schenker updated
https://github.com/llvm/llvm-project/pull/71974
>From dabfdee1a982000605e4b33930ba433c63d1684f Mon Sep 17 00:00:00 2001
From: Thomas Schenker
Date: Fri, 10 Nov 2023 18:58:26 +0100
Subject: [PATCH 1/4] [clang-tidy] bugprone-assert-side-effect non-const
https://github.com/schenker updated
https://github.com/llvm/llvm-project/pull/71974
>From dabfdee1a982000605e4b33930ba433c63d1684f Mon Sep 17 00:00:00 2001
From: Thomas Schenker
Date: Fri, 10 Nov 2023 18:58:26 +0100
Subject: [PATCH 1/4] [clang-tidy] bugprone-assert-side-effect non-const
https://github.com/schenker updated
https://github.com/llvm/llvm-project/pull/71974
>From dabfdee1a982000605e4b33930ba433c63d1684f Mon Sep 17 00:00:00 2001
From: Thomas Schenker
Date: Fri, 10 Nov 2023 18:58:26 +0100
Subject: [PATCH 1/4] [clang-tidy] bugprone-assert-side-effect non-const
@@ -41,6 +41,10 @@ AST_MATCHER_P2(Expr, hasSideEffect, bool, CheckFunctionCalls,
}
if (const auto *OpCallExpr = dyn_cast(E)) {
+if (const auto *MethodDecl =
+dyn_cast_or_null(OpCallExpr->getDirectCallee()))
+ return !MethodDecl->isConst();
@@ -41,6 +41,10 @@ AST_MATCHER_P2(Expr, hasSideEffect, bool, CheckFunctionCalls,
}
if (const auto *OpCallExpr = dyn_cast(E)) {
+if (const auto *MethodDecl =
+dyn_cast_or_null(OpCallExpr->getDirectCallee()))
+ return !MethodDecl->isConst();
schenker wrote:
> I'm not sure now if this wont cause too much false-positives, after all this
> is why CheckFunctionCalls option were added. Sometimes object can have 2 same
> operators, one const and one not, in such case depend on "this" type,
> non-const could be used, this would be
@@ -41,6 +41,10 @@ AST_MATCHER_P2(Expr, hasSideEffect, bool, CheckFunctionCalls,
}
if (const auto *OpCallExpr = dyn_cast(E)) {
+if (const auto *MethodDecl =
+dyn_cast_or_null(OpCallExpr->getDirectCallee()))
+ return !MethodDecl->isConst();
@@ -41,6 +41,10 @@ AST_MATCHER_P2(Expr, hasSideEffect, bool, CheckFunctionCalls,
}
if (const auto *OpCallExpr = dyn_cast(E)) {
+if (const auto *FuncDecl = OpCallExpr->getDirectCallee())
+ if (const auto *MethodDecl = dyn_cast(FuncDecl))
+return
https://github.com/schenker updated
https://github.com/llvm/llvm-project/pull/71974
>From dabfdee1a982000605e4b33930ba433c63d1684f Mon Sep 17 00:00:00 2001
From: Thomas Schenker
Date: Fri, 10 Nov 2023 18:58:26 +0100
Subject: [PATCH 1/3] [clang-tidy] bugprone-assert-side-effect non-const
https://github.com/PiotrZSL requested changes to this pull request.
I'm not sure now if this wont cause too much false-positives, after all this is
why CheckFunctionCalls option were added. Sometimes object can have 2 same
operators, one const and one not, in such case depend on "this" type,
@@ -41,6 +41,10 @@ AST_MATCHER_P2(Expr, hasSideEffect, bool, CheckFunctionCalls,
}
if (const auto *OpCallExpr = dyn_cast(E)) {
+if (const auto *MethodDecl =
+dyn_cast_or_null(OpCallExpr->getDirectCallee()))
+ return !MethodDecl->isConst();
PiotrZSL wrote:
One more question, there is option for this check:
"CheckFunctionCalls
Whether to treat non-const member and non-member functions as they produce side
effects. Disabled by default because it can increase the number of false
positive warnings."
Won't this overlap this change ?
https://github.com/schenker updated
https://github.com/llvm/llvm-project/pull/71974
>From dabfdee1a982000605e4b33930ba433c63d1684f Mon Sep 17 00:00:00 2001
From: Thomas Schenker
Date: Fri, 10 Nov 2023 18:58:26 +0100
Subject: [PATCH 1/2] [clang-tidy] bugprone-assert-side-effect non-const
https://github.com/schenker updated
https://github.com/llvm/llvm-project/pull/71974
>From dabfdee1a982000605e4b33930ba433c63d1684f Mon Sep 17 00:00:00 2001
From: Thomas Schenker
Date: Fri, 10 Nov 2023 18:58:26 +0100
Subject: [PATCH 1/2] [clang-tidy] bugprone-assert-side-effect non-const
@@ -41,6 +41,10 @@ AST_MATCHER_P2(Expr, hasSideEffect, bool, CheckFunctionCalls,
}
if (const auto *OpCallExpr = dyn_cast(E)) {
+if (const auto *FuncDecl = OpCallExpr->getDirectCallee())
PiotrZSL wrote:
Merge this FuncDecl with MethodDecl by using
@@ -41,6 +41,10 @@ AST_MATCHER_P2(Expr, hasSideEffect, bool, CheckFunctionCalls,
}
if (const auto *OpCallExpr = dyn_cast(E)) {
+if (const auto *FuncDecl = OpCallExpr->getDirectCallee())
+ if (const auto *MethodDecl = dyn_cast(FuncDecl))
+return
https://github.com/PiotrZSL approved this pull request.
Update documentation of this check to mention this, and update release notes.
Code looks fine.
https://github.com/llvm/llvm-project/pull/71974
___
cfe-commits mailing list
llvmbot wrote:
@llvm/pr-subscribers-clang-tidy
Author: None (schenker)
Changes
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
https://github.com/schenker created
https://github.com/llvm/llvm-project/pull/71974
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
35 matches
Mail list logo
