[clang] [analyzer][NFC] Merge `BlockInvocationContext` class into `StackFrameContext` (PR #194857)
Balázs Benics via cfe-commits
cfe-commits at lists.llvm.org
Thu Apr 30 04:56:48 PDT 2026
steakhal wrote:
> Now that I see this complete patch, I finally understand why did the old code use `void *Data` in `BlockInvocationContext`: these files (`AnalysisDeclContext.{cpp,h}`) are in a part of the clang project (a library) which is used by both the static analyzer and certain other parts of the compiler (the so-called "analysis-based warnings"). On the other hand, `BlockDataRegion` is defined in the static analyzer, so it is a reasonable idea that this more general library shouldn't know about a type that is specific to the static analyzer.
>
> The most principled approach would be retaining the `void *Data` solution to avoid mentioning an analyzer-specific type in this more general context. However, forward declaring this class increases the readability and type safety of this code and doesn't introduce a restrictive dependency relationship, so I think it is the overall better approach.
`clangAnalysis` can't depend on `clangStaticAnalyzerCore`. That would introduce a circular dependency because that already depends on `clangAnalysis`. This is a no-go. This would get reverted in minutes.
> Another significant thing is that although `LocationContext` is declared here, it is only used by the static analyzer (and `ProgramPoint` and `PathDiagnostic`, which are to my best understanding only used by the analyzer), so it would be reasonable to eventually move the type `LocationContext` (or its suggested replacement, `StackFrame`) to the static analyzer (together with stuff that depends on it). (Also, these are practically implementation details of the path sensitive analysis, I don't see a usecase for them outside of the static analyzer.)
Agreed. `LocationContext` and the new `StackFrame` are CSA concepts. I think they should live inside `clangStaticAnalyzerCore`. Probably similar reasoning applies to the other mentioned types as well.
> Note that originally the static analyzer was developed under `Analysis`, and it was only moved to the current separate `StaticAnalyzer` directory later. I think it was an oversight during this move that `LocationContext`, `ProgramPoint` and `PathDiagnostic` were not moved together with the rest of the analyzer code.
This is probable.
> (By the way, for some reason this separation was not applied among the tests, so the static analyzer tests remained under `test/Analysis` and eventually "crowded out" all the other tests. I moved the last non-analyzer tests from `test/Analysis` to other directories ~ a year ago.)
I think a split of `test/Analysis` is slowly brewing. It's not urgent or a blocker though.
https://github.com/llvm/llvm-project/pull/194857
More information about the cfe-commits
mailing list