https://github.com/spaits closed https://github.com/llvm/llvm-project/pull/75076
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
DonatNagyE wrote:
I'm also weakly opposed to this patch, because the status quo is slightly
better than introducing an assert that would make this area more fragile and
could lead to some assertion failures if this code is modified in the future.
https://github.com/llvm/llvm-project/pull/75076
https://github.com/spaits updated
https://github.com/llvm/llvm-project/pull/75076
From f3d7181073996dd94e6d0b7ac403e9a3d97f4e0d Mon Sep 17 00:00:00 2001
From: Gabor Spaits
Date: Mon, 11 Dec 2023 16:59:16 +0100
Subject: [PATCH 1/3] [Analyzer][NFC] Remove redundant function call
PRValueHandler's
@@ -2565,21 +2565,24 @@ class PRValueHandler final : public ExpressionHandler {
using ExpressionHandler::ExpressionHandler;
Tracker::Result handle(const Expr *E, const ExplodedNode *InputNode,
- const ExplodedNode *ExprNode,
+
isuckatcs wrote:
> Consider downstream users that might use this reporting system and have their
> own trackers. (We don't at Sonar, but pretend), then they would need to
> remove one more unjust assert.
This is also a fair point, though as far as I know, LLVM doesn't keep API
compatibility e
spaits wrote:
My rationale behind this change is that, as I mentioned this class is for a
very specific purpose. It is a module of sub-system, not a generic "library"
class that is intended to be used from anywhere, so this class would not really
make sense to be used anywhere else.
If we tak
https://github.com/steakhal commented:
I would agree with @isuckatcs, and I'd be weak against this PR.
Right now I don't see the benefit of asserting this.
Consider downstream users that might use this reporting system and have their
own trackers. (We don't at Sonar, but pretend), then they woul
@@ -2565,21 +2565,20 @@ class PRValueHandler final : public ExpressionHandler {
using ExpressionHandler::ExpressionHandler;
Tracker::Result handle(const Expr *E, const ExplodedNode *InputNode,
- const ExplodedNode *ExprNode,
+
https://github.com/spaits updated
https://github.com/llvm/llvm-project/pull/75076
From f3d7181073996dd94e6d0b7ac403e9a3d97f4e0d Mon Sep 17 00:00:00 2001
From: Gabor Spaits
Date: Mon, 11 Dec 2023 16:59:16 +0100
Subject: [PATCH 1/3] [Analyzer][NFC] Remove redundant function call
PRValueHandler's
@@ -2565,21 +2565,20 @@ class PRValueHandler final : public ExpressionHandler {
using ExpressionHandler::ExpressionHandler;
Tracker::Result handle(const Expr *E, const ExplodedNode *InputNode,
- const ExplodedNode *ExprNode,
+
isuckatcs wrote:
> I have also thought about a possible different caller but I think it is very
> unlikely. These `ExpressionHandler`s are basically visitor like classes for
> this tracking mechanism, if I am correct. Their purpose is to track other
> values or append `BugReporterVisitor`s to
https://github.com/spaits updated
https://github.com/llvm/llvm-project/pull/75076
From f3d7181073996dd94e6d0b7ac403e9a3d97f4e0d Mon Sep 17 00:00:00 2001
From: Gabor Spaits
Date: Mon, 11 Dec 2023 16:59:16 +0100
Subject: [PATCH 1/2] [Analyzer][NFC] Remove redundant function call
PRValueHandler's
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff a4e67de96f0a9833756b6c79fff3cd6ee459fee0
d3b3bbce9ee6fa107c6bfa183dd7885f191300d6 --
https://github.com/spaits updated
https://github.com/llvm/llvm-project/pull/75076
From f3d7181073996dd94e6d0b7ac403e9a3d97f4e0d Mon Sep 17 00:00:00 2001
From: Gabor Spaits
Date: Mon, 11 Dec 2023 16:59:16 +0100
Subject: [PATCH 1/2] [Analyzer][NFC] Remove redundant function call
PRValueHandler's
spaits wrote:
It is a great idea to use assertions here. It is exactly the context in which
they should be used. I will add that. Thank you for the review.
I have also thought about a possible different caller but I think it is very
unlikely. These `ExpressionHandler`s are basically visitor li
https://github.com/isuckatcs requested changes to this pull request.
.
https://github.com/llvm/llvm-project/pull/75076
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
isuckatcs wrote:
If it's guaranteed that `ExprNode` is for `E`, I think it would be a good idea
to assert it in the function.
```c++
assert(ExprNode->getStmtForDiagnostics() == E);
```
However I guess this assumption is made from the only currently known callsite
of the function, which is:
``
llvmbot wrote:
@llvm/pr-subscribers-clang-static-analyzer-1
Author: Gábor Spaits (spaits)
Changes
`PRValueHandler`'s handle function is only called from Tracker's track
function. In `Tracker::track` the
`ExprNode` parameter passed to `PRValueHandler::handle` is already the
`ExplodedNode`
https://github.com/spaits created
https://github.com/llvm/llvm-project/pull/75076
`PRValueHandler`'s handle function is only called from Tracker's track
function. In `Tracker::track` the
`ExprNode` parameter passed to `PRValueHandler::handle` is already the
`ExplodedNode` for the tracked expre
19 matches
Mail list logo
