This is an archive of the discontinued LLVM Phabricator instance.

[clang][dataflow] Add transfer functions for logical and, or, not.
ClosedPublic

Authored by sgatev on Feb 16 2022, 9:06 AM.

Details

Summary

This is part of the implementation of the dataflow analysis framework.
See "[RFC] A dataflow analysis framework for Clang AST" on cfe-dev.

Diff Detail

Event Timeline

sgatev created this revision.Feb 16 2022, 9:06 AM
sgatev requested review of this revision.Feb 16 2022, 9:06 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 16 2022, 9:06 AM
sgatev updated this revision to Diff 409296.Feb 16 2022, 9:24 AM

Remove unnecessary IgnoreParens calls.

xazax.hun added inline comments.Feb 16 2022, 9:42 AM
clang/include/clang/Analysis/FlowSensitive/Transfer.h
28

Depending on how willing sensitive we are to memory vs runtime, if there is one environment per basic block, we could look up the basic block of the statement, and use the basic block's id, to look up the environment in a vector.

clang/include/clang/Analysis/FlowSensitive/Value.h
39

Do we need Value in the Kind if we are already in the class Value?
Also, do we need prefix Conjunction and Disjunction with Bool? Will there be a Conjunction with other types?

79

Do we want to have a separate class for every binary/unary operator? Alternatively, we could follow what the AST is already doing. Having BinaryOperator and UnaryOperator classes with a kind. I guess the current approach is not too bad for booleans, but if we want to support all integer operators as well in the future I wonder if that would end up with too much boilerplate.

clang/lib/Analysis/FlowSensitive/Transfer.cpp
98

I wonder if this GetOrCreateFresh style operations will be common enough to have a helper for this purpose.

xazax.hun added inline comments.Feb 16 2022, 9:43 AM
clang/include/clang/Analysis/FlowSensitive/Transfer.h
28

Whoops, never mind. The implementation already reflects what I suggested.

sgatev updated this revision to Diff 409330.Feb 16 2022, 10:52 AM
sgatev marked 5 inline comments as done.

Address reviewers' comments.

clang/include/clang/Analysis/FlowSensitive/Value.h
39

We don't need the Value suffix in Kind and we don't expect other conjunctions, disjunctions, and negations for now so I simplified the names.

79

Great point! Right now we have this subdivision only for booleans and, as you mentioned, it's not too bad. Definitely worth considering the alternative design going forward. If you don't mind, I suggest starting with the current setup, with a FIXME to remind us of the other option. I'd be happy to revisit this after the next couple of patches.

clang/lib/Analysis/FlowSensitive/Transfer.cpp
98

That, or we can ensure that each expression gets assigned a value. Added a FIXME for now.

xazax.hun accepted this revision.Feb 16 2022, 11:32 AM
This revision is now accepted and ready to land.Feb 16 2022, 11:32 AM
This revision was landed with ongoing or failed builds.Feb 17 2022, 1:11 AM
This revision was automatically updated to reflect the committed changes.