https://github.com/usx95 requested changes to this pull request.
I noticed a couple of issues with the current `buildOriginFlowChain`
implementation:
**1. The `OriginFlowChain` state is polluted by BFS.**
Currently, `OriginFlowChain` is declared outside the queue. Because BFS
explores multiple branches level-by-level (e.g., both branches of an
`if/else`), `OriginFlowChain.append()` mixes the origins from *all* visited
branches into a single shared vector. This results in diagnostics emitting
alias messages for paths that weren't actually taken.
Here is a test case that reproduces the polluted chain bug:
```cpp
void test_multiple_paths(bool cond) {
View v;
{
MyObj a;
View p1, p2, p3;
p1 = a;
p2 = p1;
// Only one of the branches should be diagnosed.
if (cond) {
p3 = p1;
} else {
p3 = p2;
}
v = p3; // expected-note {{local variable 'p3' aliases...}}
} // expected-note {{destroyed here}}
v.use(); // expected-note {{later used here}}
}
```
**2. I feel DFS is a better fit here than BFS.**
BFS tries to find the shortest path, but here we don't care about the shortest
path; any valid path that tracks the origin back to the `IssueFact` is correct.
(Shortest path would anyways require Dijkstra here as each block adds arbitrary
alias notes and not just 1).
Switching to an iterative DFS (using a Stack instead of a Queue) allows the
search to dive straight down a single valid path. If we move `OriginFlowChain`
*inside* the state, we completely fix the pollution bug while keeping memory
overhead tiny, since DFS usually finds the root loan on its very first try.
https://github.com/llvm/llvm-project/pull/204592
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
