aaron.ballman added inline comments.
================
Comment at: clang/lib/ASTMatchers/Dynamic/Marshallers.h:948
+ return {};
+ auto VM = Arg.Value.getMatcher();
+ if (VM.hasTypedMatcher(NK)) {
----------------
Note, this is an example of why use of `auto` is discouraged in the project
when the type is not spelled out explicitly in the initialization -- this was
accidentally creating a non-const copy of the `VariantMatcher`, not getting a
const reference as `getMatcher()` returns. Not the worst problem in the world,
but it takes a lot of review effort to find these issues when you use `auto`,
which is why the guidance is what it is.
================
Comment at: clang/lib/ASTMatchers/Dynamic/Marshallers.h:950
+ if (VM.hasTypedMatcher(NK)) {
+ auto DM = VM.getTypedMatcher(NK);
+ InnerArgs.push_back(DM);
----------------
Rather than have a has/get pair, would it make sense to have a get method that
returns an `Optional` so that we don't have to check the same thing twice?
================
Comment at: clang/lib/ASTMatchers/Dynamic/Marshallers.h:988
+ *LeastDerivedKind = CladeNodeKind;
+ return true;
}
----------------
We used to traverse the list of matcher functions to see if one is convertible
or not and now we're always assuming that the conversion is fine -- was that
previous checking unnecessary or has it been subsumed by checking somewhere
else?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D94879/new/
https://reviews.llvm.org/D94879
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
