[PATCH] D52928: [clangd] Fix a subtle case for GetBeginningOfIdentifier.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric, ilya-biryukov.

The following case 1 was incorrectly regarded as case 2:

  MACRO(foo->^function())

Also add few more tests for macro.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52928

Files:
  clangd/ClangdUnit.cpp
  unittests/clangd/ClangdUnitTests.cpp


Index: unittests/clangd/ClangdUnitTests.cpp
===================================================================
--- unittests/clangd/ClangdUnitTests.cpp
+++ unittests/clangd/ClangdUnitTests.cpp
@@ -205,6 +205,13 @@
 }
 
 TEST(ClangdUnitTest, GetBeginningOfIdentifier) {
+  StringRef Common = R"cpp(
+struct Foo { int func(); };
+#define MACRO(X) void f() { X; }
+Foo* foo;
+Foo bar;
+)cpp";
+
   // First ^ is the expected beginning, last is the search position.
   for (const char *Text : {
            "int ^f^oo();", // inside identifier
@@ -214,15 +221,27 @@
            "^int foo();",  // beginning of file (can't back up)
            "int ^f0^0();", // after a digit (lexing at N-1 is wrong)
            "int ^λλ^λ();", // UTF-8 handled properly when backing up
+
+           // testcases where identifier is in macro.
+           "MACRO(foo->^func())", // beginning of identifier
+           "MACRO(foo->^fun^c())", // inside identifier
+           "MACRO(foo->^func^())", // end of identifier
+           "MACRO(^foo->func())", // begin identifier
+           "MACRO(^foo^->func())", // end identifier
+           "^MACRO(foo->funct())", // beginning of macro name
+           "^MAC^RO(foo->funct())", // inside macro name
+           "^MACRO^(foo->funct())", // end of macro name
        }) {
-    Annotations TestCase(Text);
+    std::string TestCode = (Common + Text).str();
+    Annotations TestCase(TestCode);
     auto AST = TestTU::withCode(TestCase.code()).build();
     const auto &SourceMgr = AST.getASTContext().getSourceManager();
     SourceLocation Actual = getBeginningOfIdentifier(
         AST, TestCase.points().back(), SourceMgr.getMainFileID());
-    Position ActualPos =
-        offsetToPosition(TestCase.code(), SourceMgr.getFileOffset(Actual));
-    EXPECT_EQ(TestCase.points().front(), ActualPos) << Text;
+    Position ActualPos = offsetToPosition(
+        TestCase.code(),
+        SourceMgr.getFileOffset(SourceMgr.getSpellingLoc(Actual)));
+    EXPECT_EQ(TestCase.points().front(), ActualPos) << TestCode;
   }
 }
 
Index: clangd/ClangdUnit.cpp
===================================================================
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -407,8 +407,15 @@
       SourceMgr.getMacroArgExpandedLocation(InputLoc.getLocWithOffset(-1));
   Before = Lexer::GetBeginningOfToken(Before, SourceMgr, AST.getLangOpts());
   Token Tok;
+  // A subtle case 1:
+  //   MACRO(foo->^function())
+  //             ^~~~~~ Before
+  // getRawToken intentionally returns the macro name (whose token is
+  // identifier), we do want the token at Before loc, to achieve that, we pass a
+  // spelling location to getRawToken.
   if (Before.isValid() &&
-      !Lexer::getRawToken(Before, Tok, SourceMgr, AST.getLangOpts(), false) &&
+      !Lexer::getRawToken(SourceMgr.getSpellingLoc(Before), Tok, SourceMgr,
+                          AST.getLangOpts(), false) &&
       Tok.is(tok::raw_identifier))
     return Before;                                        // Case 2.
   return SourceMgr.getMacroArgExpandedLocation(InputLoc); // Case 1 or 3.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D52928.168445.patch
Type: text/x-patch
Size: 3097 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20181005/78c609dc/attachment.bin>