================
@@ -171,6 +151,24 @@ class UncountedLocalVarsChecker
std::optional<bool> IsUncountedPtr = isUncountedPtr(ArgType);
if (IsUncountedPtr && *IsUncountedPtr) {
+
+ ASTContext &ctx = V->getASTContext();
+ for (DynTypedNodeList ancestors = ctx.getParents(*V); !ancestors.empty();
+ ancestors = ctx.getParents(*ancestors.begin())) {
----------------
haoNoQ wrote:
I don't think this should be a recursive loop. As is, it'll match `x` not only
in
```
if (int x = 1) {
}
```
but also in
```
if (1) {
int x;
}
```
which is probably not what you want(?) So you probably need to check that `V`
is the condition variable.
Also, generally speaking, usually we try to avoid `ParentMap`/`getParent()`
shenanigans. Parent lookups aren't naturally provided by the AST, they rely on
building the "parent map" by scanning the entire AST in the natural order and
putting all edges into the map. There are a few very confusing cornercases in
this algorithm and a few known crashes in that I honestly don't know how to
fix. It also screws algorithmic complexity because now we have an additional
backwards traversal; it's very easy to accidentally start walking back and
forth indefinitely, or just way too many times. Sometimes it's necessary but
most of the time there's a normal top-down approach that gives the answer
directly.
In this case I think it's better to redefine eg.
`VisitIfStmt()`/`TraverseIfStmt()`to manually traverse the condition variable
in a custom manner, and then suppress the automatic traversal (or just traverse
the initializer). That's clearly O(1) and doesn't rely on any unreliable
technology.
https://github.com/llvm/llvm-project/pull/82229
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
