This is an archive of the discontinued LLVM Phabricator instance.

[clang][dataflow] Add `MatchSwitch` utility library.
ClosedPublic

Authored by ymandel on Mar 3 2022, 6:00 AM.

Details

Summary

Adds MatchSwitch, a library for simplifying implementation of transfer
functions. MatchSwitch supports constructing a "switch" statement, where each
case of the switch is defined by an AST matcher. The cases are considered in
order, like pattern matching in functional languages.

Diff Detail

Event Timeline

ymandel created this revision.Mar 3 2022, 6:00 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 3 2022, 6:00 AM
ymandel requested review of this revision.Mar 3 2022, 6:00 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 3 2022, 6:00 AM
ymandel updated this revision to Diff 412693.Mar 3 2022, 6:08 AM

Adjust tests to use two different handlers.

xazax.hun added inline comments.Mar 3 2022, 9:20 AM
clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h
51

When we instantiate this with TransferState we have ASTContext both as an argument and as a member of State. Is this intentional?

110

This does not need to be addressed here but it looks like it could be a useful feature to be able to tag nodes with integer identifiers.

ymandel marked an inline comment as done.Mar 3 2022, 9:55 AM

Thanks for the fast review!

clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h
51

Yes, but...
The ASTContext is needed for the match itself, but the State type is not guaranteed to be TransferState, so won't necessarily hold the context.

But, we could reorganize a bit. Either:
a) pass MatchFinder::MatchResult, instead of BoundNodes. That would bundle the nodes, context and source manager, which seems like a good idea.
b) bake TransferState into MatchSwitch, and make the parameter genericy named rather than "lattice".

I'm inclined towards the first option, since it seems "right" to give full access to the MatchResult. WDYT?

110

Great point. I'm not sure that would work here, fwiw, because we prefix with "Tag" here to protect against interfering with any IDs in the matcher. I suppose we could "reserve" a portion of the ID space.

But, in general, integers would make more sense in other situations that deal w/ matchers. The problem is that, unless we encode them as strings, the implementation inside the matchers could be kind of messy.

ymandel updated this revision to Diff 412763.Mar 3 2022, 10:33 AM
ymandel marked an inline comment as done.

moved to MatchFinder::MatchResult.

xazax.hun accepted this revision.Mar 3 2022, 10:42 AM
xazax.hun added inline comments.
clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h
51

Option a) sounds good to me, thanks!

This revision is now accepted and ready to land.Mar 3 2022, 10:42 AM
This revision was landed with ongoing or failed builds.Mar 4 2022, 9:20 AM
This revision was automatically updated to reflect the committed changes.