[PATCH] D119745: [libTooling] Change Tranformer's consumer to take multiple changes
Yitzhak Mandelbaum via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Feb 14 11:23:22 PST 2022
ymandel accepted this revision.
ymandel added a comment.
This revision is now accepted and ready to land.
Looks good. But, please add tests. Thanks!
================
Comment at: clang/include/clang/Tooling/Transformer/Transformer.h:27
+ using ChangesConsumer = std::function<void(
+ Expected<llvm::MutableArrayRef<AtomicChange>> Changes)>;
----------------
Maybe `ChangeSetConsumer` to be more clearly a different word than `ChangeConsumer`?
================
Comment at: clang/include/clang/Tooling/Transformer/Transformer.h:28
+ using ChangesConsumer = std::function<void(
+ Expected<llvm::MutableArrayRef<AtomicChange>> Changes)>;
+
----------------
Maybe document the choice of MutableArrayRef vs passing by value? I think this is the right choice, but it's not totally obvious. My justification is that we assume most consumers are reading the data and not storing the whole array.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D119745/new/
https://reviews.llvm.org/D119745
More information about the cfe-commits
mailing list