This is an archive of the discontinued LLVM Phabricator instance.

[clang][dataflow] Add `SetupTest` parameter for `AnalysisInputs`.
ClosedPublic

Authored by wyt on Aug 22 2022, 7:33 AM.

Details

Summary

Moves the work required for retrieving annotation states into the SetupTest and PostVisitCFG callback to avoid having to run a separate pass over the CFG after analysis has completed.

Depends on D132147

Diff Detail

Event Timeline

wyt created this revision.Aug 22 2022, 7:33 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 22 2022, 7:33 AM
wyt requested review of this revision.Aug 22 2022, 7:33 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 22 2022, 7:33 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
wyt updated this revision to Diff 454527.Aug 22 2022, 8:56 AM

Propagate change from parent patch.

wyt updated this revision to Diff 454537.Aug 22 2022, 9:19 AM

Rebase.

gribozavr2 accepted this revision.Aug 22 2022, 10:51 AM
gribozavr2 added inline comments.
clang/unittests/Analysis/FlowSensitive/TestingSupport.h
106

Since SetupTest runs before PostVisitCFG, could we declare them in order?

297
304

std::move?

315

Unused variable?

317
This revision is now accepted and ready to land.Aug 22 2022, 10:51 AM
wyt updated this revision to Diff 454754.Aug 23 2022, 2:21 AM
wyt marked 5 inline comments as done.

Address comments.

gribozavr2 accepted this revision.Aug 23 2022, 2:59 AM
sgatev added inline comments.Aug 26 2022, 4:46 AM
clang/unittests/Analysis/FlowSensitive/TestingSupport.h
88

Why move this? It makes it hard to tell if there are other changes. If there are other changes, let's keep it where it is to have a clean diff and move it in a separate commit.

wyt updated this revision to Diff 456030.Aug 26 2022, 2:48 PM

Propagate change from parent patch.

gribozavr2 accepted this revision.Aug 26 2022, 3:19 PM
wyt updated this revision to Diff 456307.Aug 29 2022, 4:50 AM
wyt marked an inline comment as done.

Propagate change from parent patch.

clang/unittests/Analysis/FlowSensitive/TestingSupport.h
88

It was moved as the new parameter declared uses AnalysisOutputs, hence AnalysisOutputs needs to come before AnalysisInputs. The move is now fixed by reordering these two struct declarations in the parent patch where they were first introduced.

gribozavr2 accepted this revision.Aug 29 2022, 5:25 AM
ymandel accepted this revision.Aug 30 2022, 8:48 AM
ymandel added inline comments.
clang/unittests/Analysis/FlowSensitive/TestingSupport.h
222

formatting seems funny here?

225

no braces for a single-statement if. could also put the binding in the if condition: if (auto Error = ...

sgatev accepted this revision.Aug 30 2022, 8:53 AM
wyt updated this revision to Diff 456699.Aug 30 2022, 9:27 AM
wyt marked an inline comment as done.

Address comment.

clang/unittests/Analysis/FlowSensitive/TestingSupport.h
222

this is returned from clang-format, seems to align the arguments.

wyt edited the summary of this revision. (Show Details)Aug 30 2022, 9:39 AM
wyt updated this revision to Diff 456982.Aug 31 2022, 8:55 AM

Update according to change in parent patch to replace designated initialisers when constructing AnalysisInputs.

gribozavr2 accepted this revision.Aug 31 2022, 9:08 AM