xazax.hun accepted this revision. xazax.hun added inline comments. This revision is now accepted and ready to land. Herald added a subscriber: rnkovacs.
================ Comment at: clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h:51 +template <typename State> +using MatchSwitch = std::function<void(const Stmt &, ASTContext &, State &)>; + ---------------- ymandel wrote: > xazax.hun wrote: > > When we instantiate this with `TransferState` we have `ASTContext` both as > > an argument and as a member of `State`. Is this intentional? > 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? Option a) sounds good to me, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120900/new/ https://reviews.llvm.org/D120900 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
