li.zhe.hua created this revision.
li.zhe.hua added a reviewer: ymandel.
li.zhe.hua requested review of this revision.
Herald added a project: clang.
Previously, Transformer would invoke the consumer once per file modified per
match, in addition to any errors encountered. The consumer is not aware of which
AtomicChanges come from any particular match. It is unclear which sets of edits
may be related or whether an error invalidates any previously emitted changes.
Modify the signature of the consumer to accept a set of changes. This keeps
related changes (i.e. all edits from a single match) together, and clarifies
that errors don't produce partial changes.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D119745
Files:
clang/include/clang/Tooling/Transformer/Transformer.h
clang/lib/Tooling/Transformer/Transformer.cpp
Index: clang/lib/Tooling/Transformer/Transformer.cpp
===================================================================
--- clang/lib/Tooling/Transformer/Transformer.cpp
+++ clang/lib/Tooling/Transformer/Transformer.cpp
@@ -65,6 +65,9 @@
}
}
+ llvm::SmallVector<AtomicChange, 1> Changes;
+ Changes.reserve(ChangesByFileID.size());
for (auto &IDChangePair : ChangesByFileID)
- Consumer(std::move(IDChangePair.second));
+ Changes.push_back(std::move(IDChangePair.second));
+ Consumer(llvm::MutableArrayRef<AtomicChange>(Changes));
}
Index: clang/include/clang/Tooling/Transformer/Transformer.h
===================================================================
--- clang/include/clang/Tooling/Transformer/Transformer.h
+++ clang/include/clang/Tooling/Transformer/Transformer.h
@@ -22,16 +22,38 @@
/// defined by the arguments of the constructor.
class Transformer : public ast_matchers::MatchFinder::MatchCallback {
public:
- using ChangeConsumer =
- std::function<void(Expected<clang::tooling::AtomicChange> Change)>;
+ using ChangeConsumer = std::function<void(Expected<AtomicChange> Change)>;
+ using ChangesConsumer = std::function<void(
+ Expected<llvm::MutableArrayRef<AtomicChange>> 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<llvm::MutableArrayRef<AtomicChange>> 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, ChangesConsumer Consumer)
+ : Rule(std::move(Rule)), Consumer(std::move(Consumer)) {
+ assert(Consumer && "Consumer is empty");
+ }
/// N.B. Passes `this` pointer to `MatchFinder`. So, this object should not
/// be moved after this call.
@@ -43,8 +65,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<AtomicChange>.
+ ChangesConsumer Consumer;
};
} // namespace tooling
} // namespace clang
Index: clang/lib/Tooling/Transformer/Transformer.cpp
===================================================================
--- clang/lib/Tooling/Transformer/Transformer.cpp
+++ clang/lib/Tooling/Transformer/Transformer.cpp
@@ -65,6 +65,9 @@
}
}
+ llvm::SmallVector<AtomicChange, 1> Changes;
+ Changes.reserve(ChangesByFileID.size());
for (auto &IDChangePair : ChangesByFileID)
- Consumer(std::move(IDChangePair.second));
+ Changes.push_back(std::move(IDChangePair.second));
+ Consumer(llvm::MutableArrayRef<AtomicChange>(Changes));
}
Index: clang/include/clang/Tooling/Transformer/Transformer.h
===================================================================
--- clang/include/clang/Tooling/Transformer/Transformer.h
+++ clang/include/clang/Tooling/Transformer/Transformer.h
@@ -22,16 +22,38 @@
/// defined by the arguments of the constructor.
class Transformer : public ast_matchers::MatchFinder::MatchCallback {
public:
- using ChangeConsumer =
- std::function<void(Expected<clang::tooling::AtomicChange> Change)>;
+ using ChangeConsumer = std::function<void(Expected<AtomicChange> Change)>;
+ using ChangesConsumer = std::function<void(
+ Expected<llvm::MutableArrayRef<AtomicChange>> 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<llvm::MutableArrayRef<AtomicChange>> 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, ChangesConsumer Consumer)
+ : Rule(std::move(Rule)), Consumer(std::move(Consumer)) {
+ assert(Consumer && "Consumer is empty");
+ }
/// N.B. Passes `this` pointer to `MatchFinder`. So, this object should not
/// be moved after this call.
@@ -43,8 +65,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<AtomicChange>.
+ ChangesConsumer Consumer;
};
} // namespace tooling
} // namespace clang
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
