diff --git a/clang/include/clang/Tooling/Transformer/Transformer.h b/clang/include/clang/Tooling/Transformer/Transformer.h --- a/clang/include/clang/Tooling/Transformer/Transformer.h +++ b/clang/include/clang/Tooling/Transformer/Transformer.h @@ -22,16 +22,44 @@ /// defined by the arguments of the constructor. class Transformer : public ast_matchers::MatchFinder::MatchCallback { public: - using ChangeConsumer = - std::function Change)>; + using ChangeConsumer = std::function Change)>; + /// Provides the set of changes to the consumer. The callback is free to move + /// or destructively consume the changes as needed. + /// + /// We use \c MutableArrayRef as an abstraction to provide decoupling, and we + /// expect the majority of consumers to copy or move the individual values + /// into a separate data structure. + using ChangeSetConsumer = std::function> Changes)>; + + /// Deprecated. Use the constructor accepting \c ChangesConsumer. + /// /// \param Consumer Receives each rewrite or error. Will not necessarily be /// called for each match; for example, if the rewrite is not applicable /// because of macros, but doesn't fail. Note that clients are responsible /// for handling the case that independent \c AtomicChanges conflict with each /// other. Transformer(transformer::RewriteRule Rule, ChangeConsumer Consumer) - : Rule(std::move(Rule)), Consumer(std::move(Consumer)) {} + : Transformer(std::move(Rule), + [Consumer = std::move(Consumer)]( + Expected> Changes) { + if (Changes) + for (auto &Change : *Changes) + Consumer(std::move(Change)); + else + Consumer(Changes.takeError()); + }) {} + + /// \param Consumer Receives all rewrites for a single match, or an error. + /// Will not necessarily be called for each match; for example, if the rule + /// generates no edits but does not fail. Note that clients are responsible + /// for handling the case that independent \c AtomicChanges conflict with each + /// other. + Transformer(transformer::RewriteRule Rule, ChangeSetConsumer Consumer) + : Rule(std::move(Rule)), Consumer(std::move(Consumer)) { + assert(this->Consumer && "Consumer is empty"); + } /// N.B. Passes `this` pointer to `MatchFinder`. So, this object should not /// be moved after this call. @@ -43,8 +71,9 @@ private: transformer::RewriteRule Rule; - /// Receives each successful rewrites as an \c AtomicChange. - ChangeConsumer Consumer; + /// Receives sets of successful rewrites as an + /// \c llvm::ArrayRef. + ChangeSetConsumer Consumer; }; } // namespace tooling } // namespace clang diff --git a/clang/lib/Tooling/Transformer/Transformer.cpp b/clang/lib/Tooling/Transformer/Transformer.cpp --- a/clang/lib/Tooling/Transformer/Transformer.cpp +++ b/clang/lib/Tooling/Transformer/Transformer.cpp @@ -65,6 +65,9 @@ } } + llvm::SmallVector Changes; + Changes.reserve(ChangesByFileID.size()); for (auto &IDChangePair : ChangesByFileID) - Consumer(std::move(IDChangePair.second)); + Changes.push_back(std::move(IDChangePair.second)); + Consumer(llvm::MutableArrayRef(Changes)); } diff --git a/clang/unittests/Tooling/TransformerTest.cpp b/clang/unittests/Tooling/TransformerTest.cpp --- a/clang/unittests/Tooling/TransformerTest.cpp +++ b/clang/unittests/Tooling/TransformerTest.cpp @@ -120,10 +120,11 @@ return *ChangedCode; } - Transformer::ChangeConsumer consumer() { - return [this](Expected C) { + Transformer::ChangeSetConsumer consumer() { + return [this](Expected> C) { if (C) { - Changes.push_back(std::move(*C)); + Changes.insert(Changes.end(), std::make_move_iterator(C->begin()), + std::make_move_iterator(C->end())); } else { // FIXME: stash this error rather then printing. llvm::errs() << "Error generating changes: "