xazax.hun accepted this revision. xazax.hun added a comment. Thanks! Did you have a chance whether this makes a difference in real world scenarios? I'm mostly curious because I do not have a good mental model of how the matchers are implemented, specifically what optimizations are in place, so I don't really know how much of an impact can caching make :)
Once the rest of the comments are resolved this looks good to me. In D122121#3396149 <https://reviews.llvm.org/D122121#3396149>, @ymandel wrote: > In D122121#3396096 <https://reviews.llvm.org/D122121#3396096>, @sgatev wrote: > >> I was thinking that maybe we can split the problem in two: >> >> 1. duplicated work across `matchDynamic` calls, and >> 2. duplicated work within a single `matchDynamic` call. >> >> This patch addresses 1. as I see it as responsibility of `MatchSwitch`. Your >> comment seems to be about 2., if I'm reading it correctly. > > Right. This division makes sense, particularly the point about > responsibility. Point 2 is very much independent of `MatchSwitch`. But, I > think matching the `optional` type is about both really in that every time we > call the matcher we'll be checking the optional type, even if we only check > it once *within* a single match. Still, fixing it doen't belong in > `MatchSwitch`. +1, I like how the problem was split. I also agree that `MatchSwitch` might not be the best place to fix #2 as a more broad fix would potentially benefit other parts of Clang, like Tidy. But speaking of other parts of Clang, I wonder whether `MatchSwitch` is something that could be part of the ASTMatcher library. Some people might find it useful for implementing tools other than flow sensitive checks. Although, I do not have a strong preference, I'm completely fine leaving it where it currently is. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122121/new/ https://reviews.llvm.org/D122121 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
