Should `DataflowAnalysis::diagnoseFunction` allow to pass analysis instance created by caller?

Clang Frontend clang-tidy
1

Context of the problem:

In scope of [clang-tidy] `bugprone-unchecked-optional-access`: handle `BloombergLP::bdlb:NullableValue::makeValue` to prevent false-positives by BaLiKfromUA · Pull Request #144313 · llvm/llvm-project · GitHub I tried to extend UncheckedOptionalAccessModel to start receiving the user’s configuration during creation.

After some investigation, I found that it’s blocked since the creation of the analysis model instance is controlled by DataflowAnalysis::diagnoseFunction and not by the client code.

Construction of an analysis instance is done via the DataflowAnalysis::createAnalysis function, which is not extensible if the analysis requires custom parameters (e.g., user configuration).

Proposal

During code review of the mentioned PR, @Xazax-hun proposed to separate the creation of the analysis instance to happen before calling DataflowAnalysis::diagnoseFunction. However, currently, we are waiting for feedback from @ymand to decide if we need to approach this direction.

I am creating this topic to save the context of the conversation and unblock the original bugfix of PR.
The PR currently uses hard-coded method names as a pragmatic solution. But in long term, the goal is to make a list of method names configurable by the user.

Any additional feedback on this issue is highly appreciated!

2

Overall this sounds right, but you need to go a little further and also ask the client to pass the DataflowAnalysisContext The problem is that, right now, diagnoseFunction creates the DataflowAnalysisContext and that is a legitmate dependency for a model. (Note that the actual code passes an Environment and I think that’s a mistake. Environments are not stable across the CFG, so it seems to force an arbitrary choice of a particular environment.) You’ll also need to provide access to the solver – I would recommend adding a getter to DataflowAnalysisContext, since querying the solver (specifically for timeouts) is a standard requirement.