This is an archive of the discontinued LLVM Phabricator instance.

[dataflow] Extract arena for Value/StorageLocation out of DataflowAnalysisContext
ClosedPublic

Authored by sammccall on Apr 17 2023, 11:38 AM.

Details

Summary

DataflowAnalysisContext has a few too many responsibilities, this narrows them.

It also allows the Arena to be shared with analysis steps, which need to create
Values, without exposing the whole DACtx API (flow conditions etc).
This means Environment no longer needs to proxy all these methods.
(For now it still does, because there are many callsites to update, and maybe
if we separate bool formulas from values we can avoid churning them twice)

In future, if we untangle the concepts of Values from boolean formulas/atoms,
Arena would also be responsible for creating formulas and managing atom IDs.

Diff Detail

Event Timeline

sammccall created this revision.Apr 17 2023, 11:38 AM
Herald added a project: Restricted Project. · View Herald Transcript
sammccall requested review of this revision.Apr 17 2023, 11:38 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 17 2023, 11:38 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
gribozavr2 accepted this revision.Apr 17 2023, 12:08 PM
gribozavr2 added inline comments.
clang/include/clang/Analysis/FlowSensitive/Arena.h
16–17
This revision is now accepted and ready to land.Apr 17 2023, 12:08 PM
sgatev accepted this revision.Apr 18 2023, 12:39 AM
mboehme added inline comments.
clang/include/clang/Analysis/FlowSensitive/Arena.h
16–17

While we're at it, put Arena in backticks as well?

sammccall updated this revision to Diff 514602.Apr 18 2023, 4:28 AM

Tweak comment
Drop flow-condition substitution instead of moving into Arena, it's dead.

clang/include/clang/Analysis/FlowSensitive/Arena.h
16–17

Added triple-slashes and changed to "example" so it's less prone to getting out of date (which I think was the point?).

I kept this split as two sentences to preserve a self-contained one-line description for readability.

Quoting the name of the class itself breaks the flow of the sentence to emphasize that this is a reference, but that doesn't seem necessary in the class declaration.

gribozavr2 accepted this revision.Apr 18 2023, 4:29 AM
sgatev accepted this revision.Apr 18 2023, 6:00 AM
sgatev added inline comments.
clang/include/clang/Analysis/FlowSensitive/Arena.h
91–97

This can be removed.

ymandel accepted this revision.Apr 18 2023, 11:01 AM

Love it! Thanks.

clang/include/clang/Analysis/FlowSensitive/Arena.h
129

add newline?

xazax.hun accepted this revision.Apr 18 2023, 2:03 PM
This revision was landed with ongoing or failed builds.Apr 19 2023, 5:32 AM
This revision was automatically updated to reflect the committed changes.
sammccall marked 2 inline comments as done.