Fix fn placement around keywords (#613)

Co-authored-by: Geoff Romer <gromer@google.com>

migrate_cpp/cpp_refactoring/fn_inserter.cpp

@@ -5,6 +5,7 @@
 #include "migrate_cpp/cpp_refactoring/fn_inserter.h"
 
 #include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Lex/Lexer.h"
 
 namespace cam = ::clang::ast_matchers;
 
@@ -27,10 +28,29 @@ void FnInserter::run(const cam::MatchFinder::MatchResult& result) {
   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");
+
+  auto& sm = *(result.SourceManager);
+  auto lang_opts = result.Context->getLangOpts();
+
+  // For names like "Class::Method", replace up to "Class" not "Method".
+  clang::NestedNameSpecifierLoc qual_loc = decl->getQualifierLoc();
+  clang::SourceLocation name_begin_loc =
+      qual_loc.hasQualifier() ? qual_loc.getBeginLoc() : decl->getLocation();
+  auto range =
+      clang::CharSourceRange::getCharRange(decl->getBeginLoc(), name_begin_loc);
+
+  // In order to handle keywords like "virtual" in "virtual auto Foo() -> ...",
+  // scan the replaced text and only drop auto/void entries.
+  llvm::SmallVector<llvm::StringRef> split;
+  clang::Lexer::getSourceText(range, sm, lang_opts)
+      .split(split, ' ', /*MaxSplit=*/-1, /*KeepEmpty=*/false);
+  std::string new_text = "fn ";
+  for (llvm::StringRef t : split) {
+    if (t != "auto" && t != "void") {
+      new_text += t.str() + " ";
+    }
+  }
+  AddReplacement(*(result.SourceManager), range, new_text);
 }
 
 }  // namespace Carbon

migrate_cpp/cpp_refactoring/fn_inserter_test.cpp

@@ -23,9 +23,8 @@ TEST_F(FnInserterTest, TrailingReturn) {
 }
 
 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;";
+  constexpr char After[] = "fn inline A() -> int;";
   ExpectReplacement(Before, After);
 }
 
@@ -36,7 +35,6 @@ TEST_F(FnInserterTest, Void) {
 }
 
 TEST_F(FnInserterTest, Methods) {
-  // TODO: Need to re-lex tokens, this should probably be "fn virtual" for now.
   constexpr char Before[] = R"cpp(
     class Shape {
      public:
@@ -53,12 +51,14 @@ TEST_F(FnInserterTest, Methods) {
      private:
       double radius_;
     };
+
+    void Shape::Draw() {}
   )cpp";
   constexpr char After[] = R"(
     class Shape {
      public:
-      fn void Draw() = 0;
-      fn auto NumSides() -> int = 0;
+      fn virtual Draw() = 0;
+      fn virtual NumSides() -> int = 0;
     };
 
     class Circle : public Shape {
@@ -70,6 +70,8 @@ TEST_F(FnInserterTest, Methods) {
      private:
       double radius_;
     };
+
+    fn Shape::Draw() {}
   )";
   ExpectReplacement(Before, After);
 }

third_party/examples/woff2/carbon/include/woff2/output.carbon

@@ -34,13 +34,13 @@ class WOFF2Out {
 
   // Append n bytes of data from buf.
   // Return true if all written, false otherwise.
-  fn auto Write(buf: const void*, n: size_t) -> bool = 0;
+  fn virtual Write(buf: const void*, n: size_t) -> bool = 0;
 
   // Write n bytes of data from buf at offset.
   // Return true if all written, false otherwise.
-  fn auto Write(buf: const void*, offset: size_t, n: size_t) -> bool = 0;
+  fn virtual Write(buf: const void*, offset: size_t, n: size_t) -> bool = 0;
 
-  fn auto Size() -> size_t = 0;
+  fn virtual Size() -> size_t = 0;
 };
 
 /**

third_party/examples/woff2/carbon/src/buffer.carbon

@@ -81,7 +81,7 @@ class Buffer {
     return true;
   }
 
-  fn auto ReadU8(value: uint8_t*) -> bool {
+  fn inline ReadU8(value: uint8_t*) -> bool {
     if (offset_ + 1 > length_) {
       return FONT_COMPRESSION_FAILURE();
     }

third_party/examples/woff2/carbon/src/file.carbon

@@ -17,14 +17,14 @@ namespace woff2 {
 using std::string;
 
 
-fn auto GetFileContent(filename: const string&) -> string {
+fn inline GetFileContent(filename: const string&) -> string {
   var ifs: std::ifstream;
   return string(
     std::istreambuf_iterator<char>(ifs.rdbuf()),
     std::istreambuf_iterator<char>());
 }
 
-fn void SetFileContents(filename: const string&, start: string::iterator,
+fn inline SetFileContents(filename: const string&, start: string::iterator,
     end: string::iterator) {
   var ofs: std::ofstream;
   std::copy(start, end, std::ostream_iterator<char>(ofs));

third_party/examples/woff2/carbon/src/port.carbon

@@ -15,7 +15,7 @@ namespace woff2 {
 
 using uint32 = unsigned int;
 
-fn auto Log2Floor(n: uint32) -> int {
+fn inline Log2Floor(n: uint32) -> int {
 #if defined(__GNUC__)
   return n == 0 ? -1 : 31 ^ __builtin_clz(n);
 #else

third_party/examples/woff2/carbon/src/store_bytes.carbon

@@ -18,7 +18,7 @@
 
 namespace woff2 {
 
-fn auto StoreU32(dst: uint8_t*, offset: size_t, x: uint32_t) -> size_t {
+fn inline StoreU32(dst: uint8_t*, offset: size_t, x: uint32_t) -> size_t {
   dst[offset] = x >> 24;
   dst[offset + 1] = x >> 16;
   dst[offset + 2] = x >> 8;
@@ -26,7 +26,7 @@ fn auto StoreU32(dst: uint8_t*, offset: size_t, x: uint32_t) -> size_t {
   return offset + 4;
 }
 
-fn auto Store16(dst: uint8_t*, offset: size_t, x: int) -> size_t {
+fn inline Store16(dst: uint8_t*, offset: size_t, x: int) -> size_t {
 #if defined(WOFF_LITTLE_ENDIAN)
   *reinterpret_cast<uint16_t*>(dst + offset) =
       ((x & 0xFF) << 8) | ((x & 0xFF00) >> 8);
@@ -39,14 +39,14 @@ fn auto Store16(dst: uint8_t*, offset: size_t, x: int) -> size_t {
   return offset + 2;
 }
 
-fn void StoreU32(val: uint32_t, offset: size_t*, dst: uint8_t*) {
+fn inline StoreU32(val: uint32_t, offset: size_t*, dst: uint8_t*) {
   dst[(*offset)++] = val >> 24;
   dst[(*offset)++] = val >> 16;
   dst[(*offset)++] = val >> 8;
   dst[(*offset)++] = val;
 }
 
-fn void Store16(val: int, offset: size_t*, dst: uint8_t*) {
+fn inline Store16(val: int, offset: size_t*, dst: uint8_t*) {
 #if defined(WOFF_LITTLE_ENDIAN)
   *reinterpret_cast<uint16_t*>(dst + *offset) =
       ((val & 0xFF) << 8) | ((val & 0xFF00) >> 8);
@@ -60,7 +60,7 @@ fn void Store16(val: int, offset: size_t*, dst: uint8_t*) {
 #endif
 }
 
-fn void StoreBytes(data: const uint8_t*, len: size_t,
+fn inline StoreBytes(data: const uint8_t*, len: size_t,
                        offset: size_t*, dst: uint8_t*) {
   memcpy(&dst[*offset], data, len);
   *offset += len;