diff --git a/migrate_cpp/cpp_refactoring/BUILD b/migrate_cpp/cpp_refactoring/BUILD index ff88a2d5cbbd..843fd918a60c 100644 --- a/migrate_cpp/cpp_refactoring/BUILD +++ b/migrate_cpp/cpp_refactoring/BUILD @@ -2,7 +2,7 @@ # Exceptions. See /LICENSE for license information. # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -load("@rules_cc//cc:defs.bzl", "cc_binary") +load("@rules_cc//cc:defs.bzl", "cc_binary", "cc_library", "cc_test") package(default_visibility = ["//visibility:public"]) @@ -10,7 +10,48 @@ cc_binary( name = "cpp_refactoring", srcs = ["main.cpp"], deps = [ - "@llvm-project//clang:ast_matchers", + ":fn_inserter", "@llvm-project//clang:tooling", ], ) + +cc_library( + name = "fn_inserter", + srcs = ["fn_inserter.cpp"], + hdrs = ["fn_inserter.h"], + deps = [":matcher"], +) + +cc_test( + name = "fn_inserter_test", + srcs = ["fn_inserter_test.cpp"], + deps = [ + ":fn_inserter", + ":matcher_test_base", + "@llvm-project//clang:tooling", + "@llvm-project//llvm:gtest_main", + ], +) + +cc_library( + name = "matcher", + srcs = ["matcher.cpp"], + hdrs = ["matcher.h"], + deps = [ + "@llvm-project//clang:ast_matchers", + "@llvm-project//clang:tooling_core", + ], +) + +cc_library( + name = "matcher_test_base", + testonly = 1, + srcs = ["matcher_test_base.cpp"], + hdrs = ["matcher_test_base.h"], + deps = [ + "@llvm-project//clang:ast_matchers", + "@llvm-project//clang:tooling", + "@llvm-project//llvm:gmock", + "@llvm-project//llvm:gtest", + ], +) diff --git a/migrate_cpp/cpp_refactoring/fn_inserter.cpp b/migrate_cpp/cpp_refactoring/fn_inserter.cpp new file mode 100644 index 000000000000..f175aa76ed14 --- /dev/null +++ b/migrate_cpp/cpp_refactoring/fn_inserter.cpp @@ -0,0 +1,36 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#include "migrate_cpp/cpp_refactoring/fn_inserter.h" + +#include "clang/ASTMatchers/ASTMatchers.h" + +namespace cam = ::clang::ast_matchers; + +namespace Carbon { + +FnInserter::FnInserter(std::map& 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); +} + +void FnInserter::run(const cam::MatchFinder::MatchResult& result) { + const auto* decl = result.Nodes.getNodeAs(Label); + if (!decl) { + llvm::report_fatal_error(std::string("getNodeAs failed for ") + Label); + } + auto begin = decl->getBeginLoc(); + // Replace the first token in the range, `auto`. + auto range = clang::CharSourceRange::getTokenRange(begin, begin); + AddReplacement(*(result.SourceManager), range, "fn"); +} + +} // namespace Carbon diff --git a/migrate_cpp/cpp_refactoring/fn_inserter.h b/migrate_cpp/cpp_refactoring/fn_inserter.h new file mode 100644 index 000000000000..69cde11397c2 --- /dev/null +++ b/migrate_cpp/cpp_refactoring/fn_inserter.h @@ -0,0 +1,26 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#ifndef MIGRATE_CPP_CPP_REFACTORING_FN_INSERTER_H_ +#define MIGRATE_CPP_CPP_REFACTORING_FN_INSERTER_H_ + +#include "migrate_cpp/cpp_refactoring/matcher.h" + +namespace Carbon { + +// Inserts `fn` for functions and methods. +class FnInserter : public Matcher { + public: + explicit FnInserter(std::map& in_replacements, + MatchFinder* finder); + + void run(const MatchFinder::MatchResult& result) override; + + private: + static constexpr char Label[] = "FnInserter"; +}; + +} // namespace Carbon + +#endif // MIGRATE_CPP_CPP_REFACTORING_FN_INSERTER_H_ diff --git a/migrate_cpp/cpp_refactoring/fn_inserter_test.cpp b/migrate_cpp/cpp_refactoring/fn_inserter_test.cpp new file mode 100644 index 000000000000..fd4affda17e3 --- /dev/null +++ b/migrate_cpp/cpp_refactoring/fn_inserter_test.cpp @@ -0,0 +1,86 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#include "migrate_cpp/cpp_refactoring/fn_inserter.h" + +#include "migrate_cpp/cpp_refactoring/matcher_test_base.h" + +namespace Carbon { +namespace { + +class FnInserterTest : public MatcherTestBase { + protected: + FnInserterTest() : fn_inserter(replacements, &finder) {} + + Carbon::FnInserter fn_inserter; +}; + +TEST_F(FnInserterTest, TrailingReturn) { + constexpr char Before[] = "auto A() -> int;"; + constexpr char After[] = "fn A() -> int;"; + ExpectReplacement(Before, After); +} + +TEST_F(FnInserterTest, Inline) { + // TODO: Need to re-lex tokens, this should probably be "fn inline" for now. + constexpr char Before[] = "inline auto A() -> int;"; + constexpr char After[] = "fn auto A() -> int;"; + ExpectReplacement(Before, After); +} + +TEST_F(FnInserterTest, Void) { + // TODO: void needs to be handled. + constexpr char Before[] = "void A();"; + ExpectReplacement(Before, Before); +} + +TEST_F(FnInserterTest, Methods) { + // TODO: void needs to be handled. + // TODO: Need to re-lex tokens, this should probably be "fn virtual" for now. + constexpr char Before[] = R"cpp( + class Shape { + public: + virtual void Draw() = 0; + virtual auto NumSides() -> int = 0; + }; + + class Circle : public Shape { + public: + void Draw() override; + auto NumSides() -> int override; + auto Radius() -> double { return radius_; } + + private: + double radius_; + }; + )cpp"; + constexpr char After[] = R"( + class Shape { + public: + virtual void Draw() = 0; + fn auto NumSides() -> int = 0; + }; + + class Circle : public Shape { + public: + void Draw() override; + fn NumSides() -> int override; + fn Radius() -> double { return radius_; } + + private: + double radius_; + }; + )"; + ExpectReplacement(Before, After); +} + +TEST_F(FnInserterTest, LegacyReturn) { + // Code should be migrated to trailing returns by clang-tidy, so this is okay + // to miss. + constexpr char Before[] = "int A();"; + ExpectReplacement(Before, Before); +} + +} // namespace +} // namespace Carbon diff --git a/migrate_cpp/cpp_refactoring/main.cpp b/migrate_cpp/cpp_refactoring/main.cpp index d62985906a81..f8178f60844f 100644 --- a/migrate_cpp/cpp_refactoring/main.cpp +++ b/migrate_cpp/cpp_refactoring/main.cpp @@ -2,90 +2,17 @@ // Exceptions. See /LICENSE for license information. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -#include "clang/ASTMatchers/ASTMatchFinder.h" -#include "clang/ASTMatchers/ASTMatchers.h" #include "clang/Tooling/CommonOptionsParser.h" #include "clang/Tooling/Refactoring.h" +#include "migrate_cpp/cpp_refactoring/fn_inserter.h" namespace cam = ::clang::ast_matchers; namespace ct = ::clang::tooling; -namespace Carbon { - -class Matcher : public cam::MatchFinder::MatchCallback { - public: - explicit Matcher(std::map& in_replacements) - : replacements(&in_replacements) {} - - ~Matcher() override = default; - - void AddReplacement(const clang::SourceManager& sm, - clang::CharSourceRange range, - llvm::StringRef replacement_text) { - if (!range.isValid()) { - llvm::errs() << "Invalid range: " << range.getAsRange().printToString(sm) - << "\n"; - return; - } - if (sm.getDecomposedLoc(range.getBegin()).first != - sm.getDecomposedLoc(range.getEnd()).first) { - llvm::errs() << "Range spans macro expansions: " - << range.getAsRange().printToString(sm) << "\n"; - return; - } - if (sm.getFileID(range.getBegin()) != sm.getFileID(range.getEnd())) { - llvm::errs() << "Range spans files: " - << range.getAsRange().printToString(sm) << "\n"; - return; - } - - auto rep = - ct::Replacement(sm, sm.getExpansionRange(range), replacement_text); - auto err = (*replacements)[std::string(rep.getFilePath())].add(rep); - if (err) { - llvm::report_fatal_error("Error with replacement `" + rep.toString() + - "`: " + llvm::toString(std::move(err)) + "\n"); - } - } - - private: - std::map* replacements; -}; - -class FnInserter : public Matcher { - public: - explicit FnInserter(std::map& in_replacements, - cam::MatchFinder* finder) - : Matcher(in_replacements) { - finder->addMatcher(cam::functionDecl(cam::isExpansionInMainFile(), - cam::hasTrailingReturn()) - .bind(Label), - this); - } - - void run(const cam::MatchFinder::MatchResult& result) override { - // The matched 'if' statement was bound to 'ifStmt'. - const auto* decl = result.Nodes.getNodeAs(Label); - if (!decl) { - llvm::report_fatal_error(std::string("getNodeAs failed for ") + Label); - } - auto begin = decl->getBeginLoc(); - // Replace the first token in the range, `auto`. - auto range = clang::CharSourceRange::getTokenRange(begin, begin); - AddReplacement(*(result.SourceManager), range, "fn"); - } - - private: - static constexpr char Label[] = "FnInserter"; -}; - -} // namespace Carbon - auto main(int argc, const char** argv) -> int { llvm::cl::OptionCategory category("C++ refactoring options"); - clang::tooling::CommonOptionsParser op(argc, argv, category); - clang::tooling::RefactoringTool tool(op.getCompilations(), - op.getSourcePathList()); + ct::CommonOptionsParser op(argc, argv, category); + ct::RefactoringTool tool(op.getCompilations(), op.getSourcePathList()); // Set up AST matcher callbacks. cam::MatchFinder finder; diff --git a/migrate_cpp/cpp_refactoring/matcher.cpp b/migrate_cpp/cpp_refactoring/matcher.cpp new file mode 100644 index 000000000000..e67fc13760c0 --- /dev/null +++ b/migrate_cpp/cpp_refactoring/matcher.cpp @@ -0,0 +1,39 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#include "migrate_cpp/cpp_refactoring/matcher.h" + +namespace ct = ::clang::tooling; + +namespace Carbon { + +void Matcher::AddReplacement(const clang::SourceManager& sm, + clang::CharSourceRange range, + llvm::StringRef replacement_text) { + if (!range.isValid()) { + llvm::errs() << "Invalid range: " << range.getAsRange().printToString(sm) + << "\n"; + return; + } + if (sm.getDecomposedLoc(range.getBegin()).first != + sm.getDecomposedLoc(range.getEnd()).first) { + llvm::errs() << "Range spans macro expansions: " + << range.getAsRange().printToString(sm) << "\n"; + return; + } + if (sm.getFileID(range.getBegin()) != sm.getFileID(range.getEnd())) { + llvm::errs() << "Range spans files: " + << range.getAsRange().printToString(sm) << "\n"; + return; + } + + auto rep = ct::Replacement(sm, sm.getExpansionRange(range), replacement_text); + auto err = (*replacements)[std::string(rep.getFilePath())].add(rep); + if (err) { + llvm::report_fatal_error("Error with replacement `" + rep.toString() + + "`: " + llvm::toString(std::move(err)) + "\n"); + } +} + +} // namespace Carbon diff --git a/migrate_cpp/cpp_refactoring/matcher.h b/migrate_cpp/cpp_refactoring/matcher.h new file mode 100644 index 000000000000..31db93c12c5c --- /dev/null +++ b/migrate_cpp/cpp_refactoring/matcher.h @@ -0,0 +1,35 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#ifndef MIGRATE_CPP_CPP_REFACTORING_MATCHER_H_ +#define MIGRATE_CPP_CPP_REFACTORING_MATCHER_H_ + +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Tooling/Core/Replacement.h" + +namespace Carbon { + +// This is an abstract class with helpers to make it easier to write matchers. +class Matcher : public clang::ast_matchers::MatchFinder::MatchCallback { + public: + // Alias these to elide the namespaces in subclass headers. + using MatchFinder = clang::ast_matchers::MatchFinder; + using Replacements = clang::tooling::Replacements; + + explicit Matcher(std::map& in_replacements) + : replacements(&in_replacements) {} + + protected: + // Replaces the given range with the specified text. + void AddReplacement(const clang::SourceManager& sm, + clang::CharSourceRange range, + llvm::StringRef replacement_text); + + private: + std::map* const replacements; +}; + +} // namespace Carbon + +#endif // MIGRATE_CPP_CPP_REFACTORING_MATCHER_H_ diff --git a/migrate_cpp/cpp_refactoring/matcher_test_base.cpp b/migrate_cpp/cpp_refactoring/matcher_test_base.cpp new file mode 100644 index 000000000000..57cc1153cbe1 --- /dev/null +++ b/migrate_cpp/cpp_refactoring/matcher_test_base.cpp @@ -0,0 +1,38 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#include "migrate_cpp/cpp_refactoring/matcher_test_base.h" + +#include "clang/Tooling/Tooling.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +namespace ct = ::clang::tooling; + +namespace Carbon { + +void MatcherTestBase::ExpectReplacement(llvm::StringRef before, + llvm::StringRef after) { + auto factory = ct::newFrontendActionFactory(&finder); + constexpr char Filename[] = "test.cc"; + ASSERT_TRUE(ct::runToolOnCodeWithArgs( + factory->create(), before, {}, Filename, "clang-tool", + std::make_shared(), + 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 actual_lines; + llvm::SplitString(*actual, actual_lines, "\n"); + llvm::SmallVector 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)); + } +} + +} // namespace Carbon diff --git a/migrate_cpp/cpp_refactoring/matcher_test_base.h b/migrate_cpp/cpp_refactoring/matcher_test_base.h new file mode 100644 index 000000000000..74f974220467 --- /dev/null +++ b/migrate_cpp/cpp_refactoring/matcher_test_base.h @@ -0,0 +1,27 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#ifndef MIGRATE_CPP_CPP_REFACTORING_MATCHER_TEST_BASE_H_ +#define MIGRATE_CPP_CPP_REFACTORING_MATCHER_TEST_BASE_H_ + +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Tooling/Core/Replacement.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +namespace Carbon { + +class MatcherTestBase : public ::testing::Test { + protected: + // Expects that that the replacements produced by running the finder result in + // the specified code transformation. + void ExpectReplacement(llvm::StringRef before, llvm::StringRef after); + + std::map replacements; + clang::ast_matchers::MatchFinder finder; +}; + +} // namespace Carbon + +#endif // MIGRATE_CPP_CPP_REFACTORING_MATCHER_TEST_BASE_H_