Skip to content

Commit

Permalink
Change how replacements are filtered (#541)
Browse files Browse the repository at this point in the history
This replaces the use of `isExpansionInMainFile` with instead taking the list of input files, and only touching them. This doesn't keep `isExpansionInMainFile` because it should be redundant, and as such it'd be easy to forget. Unless it starts being a performance issue, it's probably better to omit.
  • Loading branch information
jonmeow authored Jun 3, 2021
1 parent 8b57b9f commit af2136e
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 21 deletions.
9 changes: 2 additions & 7 deletions migrate_cpp/cpp_refactoring/fn_inserter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,8 @@ namespace Carbon {
FnInserter::FnInserter(std::map<std::string, Replacements>& in_replacements,
cam::MatchFinder* finder)
: Matcher(in_replacements) {
// TODO: Switch from isExpansionInMainFile to isDefinition. That should then
// include `for (const auto* redecl : func->redecls())` to generate
// replacements.
finder->addMatcher(
cam::functionDecl(cam::isExpansionInMainFile(), cam::hasTrailingReturn())
.bind(Label),
this);
finder->addMatcher(cam::functionDecl(cam::hasTrailingReturn()).bind(Label),
this);
}

void FnInserter::run(const cam::MatchFinder::MatchResult& result) {
Expand Down
16 changes: 16 additions & 0 deletions migrate_cpp/cpp_refactoring/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,27 @@
namespace cam = ::clang::ast_matchers;
namespace ct = ::clang::tooling;

// Initialize the files in replacements. Matcher will restrict replacements to
// initialized files.
static void InitReplacements(ct::RefactoringTool* tool) {
auto& files = tool->getFiles();
auto& repl = tool->getReplacements();
for (const auto& path : tool->getSourcePaths()) {
auto file = files.getFile(path);
if (file.getError()) {
llvm::report_fatal_error("Error accessing `" + path +
"`: " + file.getError().message() + "\n");
}
repl.insert({files.getCanonicalName(*file).str(), {}});
}
}

auto main(int argc, const char** argv) -> int {
llvm::cl::OptionCategory category("C++ refactoring options");
auto parser = ct::CommonOptionsParser::create(argc, argv, category);
ct::RefactoringTool tool(parser->getCompilations(),
parser->getSourcePathList());
InitReplacements(&tool);

// Set up AST matcher callbacks.
cam::MatchFinder finder;
Expand Down
9 changes: 8 additions & 1 deletion migrate_cpp/cpp_refactoring/matcher.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,14 @@ void Matcher::AddReplacement(const clang::SourceManager& sm,
}

auto rep = ct::Replacement(sm, sm.getExpansionRange(range), replacement_text);
auto err = (*replacements)[std::string(rep.getFilePath())].add(rep);
auto entry = replacements->find(std::string(rep.getFilePath()));
if (entry == replacements->end()) {
// The replacement was in a file which isn't being updated, such as a system
// header.
return;
}

auto err = entry->second.add(rep);
if (err) {
llvm::report_fatal_error("Error with replacement `" + rep.toString() +
"`: " + llvm::toString(std::move(err)) + "\n");
Expand Down
23 changes: 10 additions & 13 deletions migrate_cpp/cpp_refactoring/matcher_test_base.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,23 +16,20 @@ void MatcherTestBase::ExpectReplacement(llvm::StringRef before,
llvm::StringRef after) {
auto factory = ct::newFrontendActionFactory(&finder);
constexpr char Filename[] = "test.cc";
replacements.clear();
replacements.insert({Filename, {}});
ASSERT_TRUE(ct::runToolOnCodeWithArgs(
factory->create(), before, {}, Filename, "clang-tool",
std::make_shared<clang::PCHContainerOperations>(),
ct::FileContentMappings()));
auto it = replacements.find(Filename);
if (it != replacements.end()) {
auto actual = ct::applyAllReplacements(before, it->second);
// Split lines to get gmock to get an easier-to-read error.
llvm::SmallVector<llvm::StringRef, 0> actual_lines;
llvm::SplitString(*actual, actual_lines, "\n");
llvm::SmallVector<llvm::StringRef, 0> after_lines;
llvm::SplitString(after, after_lines, "\n");
EXPECT_THAT(actual_lines, testing::ContainerEq(after_lines));
} else {
// No replacements; before and after should match.
EXPECT_THAT(before, testing::Eq(after));
}
EXPECT_THAT(replacements, testing::ElementsAre(testing::Key(Filename)));
auto actual = ct::applyAllReplacements(before, replacements[Filename]);
// Split lines to get gmock to get an easier-to-read error.
llvm::SmallVector<llvm::StringRef, 0> actual_lines;
llvm::SplitString(*actual, actual_lines, "\n");
llvm::SmallVector<llvm::StringRef, 0> after_lines;
llvm::SplitString(after, after_lines, "\n");
EXPECT_THAT(actual_lines, testing::ContainerEq(after_lines));
}

} // namespace Carbon

0 comments on commit af2136e

Please sign in to comment.