li.zhe.hua updated this revision to Diff 408699.
li.zhe.hua added a comment.
Attempt fix for Windows compilation issue
Work around ambiguous function call by foregoing the delgating constructor.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D119745/new/
https://reviews.llvm.org/D119745
Files:
clang/include/clang/Tooling/Transformer/Transformer.h
clang/lib/Tooling/Transformer/Transformer.cpp
clang/unittests/Tooling/TransformerTest.cpp
Index: clang/unittests/Tooling/TransformerTest.cpp
===================================================================
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -26,6 +26,7 @@
using ::clang::transformer::before;
using ::clang::transformer::cat;
using ::clang::transformer::changeTo;
+using ::clang::transformer::editList;
using ::clang::transformer::makeRule;
using ::clang::transformer::member;
using ::clang::transformer::name;
@@ -36,6 +37,8 @@
using ::clang::transformer::statement;
using ::testing::ElementsAre;
using ::testing::IsEmpty;
+using ::testing::ResultOf;
+using ::testing::UnorderedElementsAre;
constexpr char KHeaderContents[] = R"cc(
struct string {
@@ -120,10 +123,11 @@
return *ChangedCode;
}
- Transformer::ChangeConsumer consumer() {
- return [this](Expected<AtomicChange> C) {
+ Transformer::ChangeSetConsumer consumer() {
+ return [this](Expected<MutableArrayRef<AtomicChange>> 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: "
@@ -799,7 +803,6 @@
}
TEST_F(TransformerTest, EditList) {
- using clang::transformer::editList;
std::string Input = R"cc(
void foo() {
if (10 > 1.0)
@@ -827,7 +830,6 @@
}
TEST_F(TransformerTest, Flatten) {
- using clang::transformer::editList;
std::string Input = R"cc(
void foo() {
if (10 > 1.0)
@@ -1638,4 +1640,46 @@
<< "Could not update code: " << llvm::toString(UpdatedCode.takeError());
EXPECT_EQ(format(*UpdatedCode), format(Header));
}
+
+// A single change set can span multiple files.
+TEST_F(TransformerTest, MultiFileEdit) {
+ // NB: The fixture is unused for this test, but kept for the test suite name.
+ std::string Header = R"cc(void Func(int id);)cc";
+ std::string Source = R"cc(#include "input.h"
+ void Caller() {
+ int id = 0;
+ Func(id);
+ })cc";
+ int ErrorCount = 0;
+ std::vector<AtomicChanges> ChangeSets;
+ clang::ast_matchers::MatchFinder MatchFinder;
+ Transformer T(
+ makeRule(callExpr(callee(functionDecl(hasName("Func"))),
+ forEachArgumentWithParam(expr().bind("arg"),
+ parmVarDecl().bind("param"))),
+ editList({changeTo(node("arg"), cat("ARG")),
+ changeTo(node("param"), cat("PARAM"))})),
+ [&](Expected<MutableArrayRef<AtomicChange>> Changes) {
+ if (Changes)
+ ChangeSets.push_back(AtomicChanges(Changes->begin(), Changes->end()));
+ else
+ ++ErrorCount;
+ });
+ T.registerMatchers(&MatchFinder);
+ auto Factory = newFrontendActionFactory(&MatchFinder);
+ EXPECT_TRUE(runToolOnCodeWithArgs(
+ Factory->create(), Source, std::vector<std::string>(), "input.cc",
+ "clang-tool", std::make_shared<PCHContainerOperations>(),
+ {{"input.h", Header}}));
+
+ EXPECT_EQ(ErrorCount, 0);
+ EXPECT_THAT(
+ ChangeSets,
+ UnorderedElementsAre(UnorderedElementsAre(
+ ResultOf([](const AtomicChange &C) { return C.getFilePath(); },
+ "input.cc"),
+ ResultOf([](const AtomicChange &C) { return C.getFilePath(); },
+ "./input.h"))));
+}
+
} // namespace
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,44 @@
/// 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)>;
+ /// 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<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)) {}
+ : Rule(std::move(Rule)),
+ Consumer([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, 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<AtomicChange>.
+ ChangeSetConsumer Consumer;
};
} // namespace tooling
} // namespace clang
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
