[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