Author: sammccall Date: Fri Oct 5 05:08:06 2018 New Revision: 343844 URL: http://llvm.org/viewvc/llvm-project?rev=343844&view=rev Log: [clangd] Fix a subtle case for GetBeginningOfIdentifier.
Calling getMacroArgExpansionLocation too early was causing Lexer::getRawToken to do the wrong thing - lexing the macro name instead of the arg contents. Differential Revision: https://reviews.llvm.org/D52928 Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=343844&r1=343843&r2=343844&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original) +++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Fri Oct 5 05:08:06 2018 @@ -390,7 +390,6 @@ SourceLocation clangd::getBeginningOfIde log("getBeginningOfIdentifier: {0}", Offset.takeError()); return SourceLocation(); } - SourceLocation InputLoc = SourceMgr.getComposedLoc(FID, *Offset); // GetBeginningOfToken(pos) is almost what we want, but does the wrong thing // if the cursor is at the end of the identifier. @@ -401,15 +400,16 @@ SourceLocation clangd::getBeginningOfIde // 3) anywhere outside an identifier, we'll get some non-identifier thing. // We can't actually distinguish cases 1 and 3, but returning the original // location is correct for both! + SourceLocation InputLoc = SourceMgr.getComposedLoc(FID, *Offset); if (*Offset == 0) // Case 1 or 3. return SourceMgr.getMacroArgExpandedLocation(InputLoc); - SourceLocation Before = - SourceMgr.getMacroArgExpandedLocation(InputLoc.getLocWithOffset(-1)); + SourceLocation Before = SourceMgr.getComposedLoc(FID, *Offset - 1); + Before = Lexer::GetBeginningOfToken(Before, SourceMgr, AST.getLangOpts()); Token Tok; if (Before.isValid() && !Lexer::getRawToken(Before, Tok, SourceMgr, AST.getLangOpts(), false) && Tok.is(tok::raw_identifier)) - return Before; // Case 2. + return SourceMgr.getMacroArgExpandedLocation(Before); // Case 2. return SourceMgr.getMacroArgExpandedLocation(InputLoc); // Case 1 or 3. } Modified: clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp?rev=343844&r1=343843&r2=343844&view=diff ============================================================================== --- clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp (original) +++ clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp Fri Oct 5 05:08:06 2018 @@ -205,8 +205,13 @@ main.cpp:2:3: error: something terrible } TEST(ClangdUnitTest, GetBeginningOfIdentifier) { + std::string Preamble = R"cpp( +struct Bar { int func(); }; +#define MACRO(X) void f() { X; } +Bar* bar; + )cpp"; // First ^ is the expected beginning, last is the search position. - for (const char *Text : { + for (std::string Text : std::vector<std::string>{ "int ^f^oo();", // inside identifier "int ^foo();", // beginning of identifier "int ^foo^();", // end of identifier @@ -214,14 +219,26 @@ TEST(ClangdUnitTest, GetBeginningOfIdent "^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 + + // identifier in macro arg + "MACRO(bar->^func())", // beginning of identifier + "MACRO(bar->^fun^c())", // inside identifier + "MACRO(bar->^func^())", // end of identifier + "MACRO(^bar->func())", // begin identifier + "MACRO(^bar^->func())", // end identifier + "^MACRO(bar->func())", // beginning of macro name + "^MAC^RO(bar->func())", // inside macro name + "^MACRO^(bar->func())", // end of macro name }) { - Annotations TestCase(Text); + std::string WithPreamble = Preamble + Text; + Annotations TestCase(WithPreamble); 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)); + Position ActualPos = offsetToPosition( + TestCase.code(), + SourceMgr.getFileOffset(SourceMgr.getSpellingLoc(Actual))); EXPECT_EQ(TestCase.points().front(), ActualPos) << Text; } } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
