diff options
-rw-r--r-- | clang-tools-extra/clangd/ClangdServer.cpp | 2 | ||||
-rw-r--r-- | clang-tools-extra/clangd/ClangdServer.h | 1 | ||||
-rw-r--r-- | clang-tools-extra/clangd/ClangdUnit.cpp | 33 | ||||
-rw-r--r-- | clang-tools-extra/clangd/ClangdUnit.h | 5 | ||||
-rw-r--r-- | clang-tools-extra/clangd/CodeComplete.h | 2 | ||||
-rw-r--r-- | clang-tools-extra/clangd/Selection.cpp | 2 | ||||
-rw-r--r-- | clang-tools-extra/clangd/Selection.h | 1 | ||||
-rw-r--r-- | clang-tools-extra/clangd/SourceCode.cpp | 33 | ||||
-rw-r--r-- | clang-tools-extra/clangd/SourceCode.h | 7 | ||||
-rw-r--r-- | clang-tools-extra/clangd/XRefs.cpp | 23 | ||||
-rw-r--r-- | clang-tools-extra/clangd/index/FileIndex.h | 3 | ||||
-rw-r--r-- | clang-tools-extra/clangd/refactor/Rename.cpp | 6 | ||||
-rw-r--r-- | clang-tools-extra/clangd/refactor/Rename.h | 4 | ||||
-rw-r--r-- | clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp | 39 | ||||
-rw-r--r-- | clang-tools-extra/clangd/unittests/SourceCodeTests.cpp | 43 |
15 files changed, 105 insertions, 99 deletions
diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp index c4ecc5286ba..01cac6e5dc1 100644 --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -315,7 +315,7 @@ void ClangdServer::prepareRename(PathRef File, Position Pos, return CB(Changes.takeError()); } SourceLocation Loc = getBeginningOfIdentifier( - AST, Pos, AST.getSourceManager().getMainFileID()); + Pos, AST.getSourceManager(), AST.getASTContext().getLangOpts()); if (auto Range = getTokenRange(AST.getSourceManager(), AST.getASTContext().getLangOpts(), Loc)) return CB(*Range); diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h index f6f07f5fdd7..83a74ff54c5 100644 --- a/clang-tools-extra/clangd/ClangdServer.h +++ b/clang-tools-extra/clangd/ClangdServer.h @@ -11,7 +11,6 @@ #include "../clang-tidy/ClangTidyOptions.h" #include "Cancellation.h" -#include "ClangdUnit.h" #include "CodeComplete.h" #include "FSProvider.h" #include "FormattedString.h" diff --git a/clang-tools-extra/clangd/ClangdUnit.cpp b/clang-tools-extra/clangd/ClangdUnit.cpp index 4265d7e6527..bc4ec4b71a6 100644 --- a/clang-tools-extra/clangd/ClangdUnit.cpp +++ b/clang-tools-extra/clangd/ClangdUnit.cpp @@ -710,39 +710,6 @@ buildAST(PathRef FileName, std::unique_ptr<CompilerInvocation> Invocation, std::move(VFS), Inputs.Index, Inputs.Opts); } -SourceLocation getBeginningOfIdentifier(const ParsedAST &Unit, - const Position &Pos, const FileID FID) { - const ASTContext &AST = Unit.getASTContext(); - const SourceManager &SourceMgr = AST.getSourceManager(); - auto Offset = positionToOffset(SourceMgr.getBufferData(FID), Pos); - if (!Offset) { - log("getBeginningOfIdentifier: {0}", Offset.takeError()); - return SourceLocation(); - } - - // GetBeginningOfToken(pos) is almost what we want, but does the wrong thing - // if the cursor is at the end of the identifier. - // Instead, we lex at GetBeginningOfToken(pos - 1). The cases are: - // 1) at the beginning of an identifier, we'll be looking at something - // that isn't an identifier. - // 2) at the middle or end of an identifier, we get the identifier. - // 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.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 SourceMgr.getMacroArgExpandedLocation(Before); // Case 2. - return SourceMgr.getMacroArgExpandedLocation(InputLoc); // Case 1 or 3. -} - } // namespace clangd namespace tidy { // Force the linker to link in Clang-tidy modules. diff --git a/clang-tools-extra/clangd/ClangdUnit.h b/clang-tools-extra/clangd/ClangdUnit.h index a735430da4b..6fc87775d2f 100644 --- a/clang-tools-extra/clangd/ClangdUnit.h +++ b/clang-tools-extra/clangd/ClangdUnit.h @@ -187,11 +187,6 @@ buildAST(PathRef FileName, std::unique_ptr<CompilerInvocation> Invocation, const ParseInputs &Inputs, std::shared_ptr<const PreambleData> Preamble); -/// Get the beginning SourceLocation at a specified \p Pos. -/// May be invalid if Pos is, or if there's no identifier. -SourceLocation getBeginningOfIdentifier(const ParsedAST &Unit, - const Position &Pos, const FileID FID); - /// For testing/debugging purposes. Note that this method deserializes all /// unserialized Decls, so use with care. void dumpAST(ParsedAST &AST, llvm::raw_ostream &OS); diff --git a/clang-tools-extra/clangd/CodeComplete.h b/clang-tools-extra/clangd/CodeComplete.h index cf872729df9..381c97e4aad 100644 --- a/clang-tools-extra/clangd/CodeComplete.h +++ b/clang-tools-extra/clangd/CodeComplete.h @@ -15,7 +15,6 @@ #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_CODECOMPLETE_H #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_CODECOMPLETE_H -#include "ClangdUnit.h" #include "Headers.h" #include "Logger.h" #include "Path.h" @@ -36,6 +35,7 @@ namespace clang { class NamedDecl; namespace clangd { +struct PreambleData; struct CodeCompleteOptions { /// Returns options that can be passed to clang's completion engine. diff --git a/clang-tools-extra/clangd/Selection.cpp b/clang-tools-extra/clangd/Selection.cpp index cd1c84b43c7..77174936dfe 100644 --- a/clang-tools-extra/clangd/Selection.cpp +++ b/clang-tools-extra/clangd/Selection.cpp @@ -7,7 +7,6 @@ //===----------------------------------------------------------------------===// #include "Selection.h" -#include "ClangdUnit.h" #include "Logger.h" #include "SourceCode.h" #include "clang/AST/ASTTypeTraits.h" @@ -17,6 +16,7 @@ #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "clang/Basic/TokenKinds.h" +#include "clang/Lex/Lexer.h" #include "clang/Tooling/Syntax/Tokens.h" #include "llvm/ADT/STLExtras.h" #include "llvm/Support/raw_ostream.h" diff --git a/clang-tools-extra/clangd/Selection.h b/clang-tools-extra/clangd/Selection.h index 7853759a17e..9bcb9d5fb01 100644 --- a/clang-tools-extra/clangd/Selection.h +++ b/clang-tools-extra/clangd/Selection.h @@ -40,7 +40,6 @@ namespace clang { namespace clangd { -class ParsedAST; // A selection can partially or completely cover several AST nodes. // The SelectionTree contains nodes that are covered, and their parents. diff --git a/clang-tools-extra/clangd/SourceCode.cpp b/clang-tools-extra/clangd/SourceCode.cpp index 0c52dcd6715..42910163bd9 100644 --- a/clang-tools-extra/clangd/SourceCode.cpp +++ b/clang-tools-extra/clangd/SourceCode.cpp @@ -229,6 +229,39 @@ llvm::Optional<Range> getTokenRange(const SourceManager &SM, return halfOpenToRange(SM, CharSourceRange::getCharRange(TokLoc, End)); } +SourceLocation getBeginningOfIdentifier(const Position &Pos, + const SourceManager &SM, + const LangOptions &LangOpts) { + FileID FID = SM.getMainFileID(); + auto Offset = positionToOffset(SM.getBufferData(FID), Pos); + if (!Offset) { + log("getBeginningOfIdentifier: {0}", Offset.takeError()); + return SourceLocation(); + } + + // GetBeginningOfToken(pos) is almost what we want, but does the wrong thing + // if the cursor is at the end of the identifier. + // Instead, we lex at GetBeginningOfToken(pos - 1). The cases are: + // 1) at the beginning of an identifier, we'll be looking at something + // that isn't an identifier. + // 2) at the middle or end of an identifier, we get the identifier. + // 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 = SM.getComposedLoc(FID, *Offset); + if (*Offset == 0) // Case 1 or 3. + return SM.getMacroArgExpandedLocation(InputLoc); + SourceLocation Before = SM.getComposedLoc(FID, *Offset - 1); + + Before = Lexer::GetBeginningOfToken(Before, SM, LangOpts); + Token Tok; + if (Before.isValid() && + !Lexer::getRawToken(Before, Tok, SM, LangOpts, false) && + Tok.is(tok::raw_identifier)) + return SM.getMacroArgExpandedLocation(Before); // Case 2. + return SM.getMacroArgExpandedLocation(InputLoc); // Case 1 or 3. +} + bool isValidFileRange(const SourceManager &Mgr, SourceRange R) { if (!R.getBegin().isValid() || !R.getEnd().isValid()) return false; diff --git a/clang-tools-extra/clangd/SourceCode.h b/clang-tools-extra/clangd/SourceCode.h index 4e207069569..5f27ee2be21 100644 --- a/clang-tools-extra/clangd/SourceCode.h +++ b/clang-tools-extra/clangd/SourceCode.h @@ -75,6 +75,13 @@ llvm::Optional<Range> getTokenRange(const SourceManager &SM, llvm::Expected<SourceLocation> sourceLocationInMainFile(const SourceManager &SM, Position P); +/// Get the beginning SourceLocation at a specified \p Pos in the main file. +/// May be invalid if Pos is, or if there's no identifier. +/// FIXME: this returns the macro-expansion location, but it shouldn't. +SourceLocation getBeginningOfIdentifier(const Position &Pos, + const SourceManager &SM, + const LangOptions &LangOpts); + /// Returns true iff \p Loc is inside the main file. This function handles /// file & macro locations. For macro locations, returns iff the macro is being /// expanded inside the main file. diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index dc69ef76038..1b500c9398b 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -254,8 +254,8 @@ std::vector<LocatedSymbol> locateSymbolAt(ParsedAST &AST, Position Pos, } } - SourceLocation SourceLocationBeg = - getBeginningOfIdentifier(AST, Pos, SM.getMainFileID()); + SourceLocation SourceLocationBeg = getBeginningOfIdentifier( + Pos, AST.getSourceManager(), AST.getASTContext().getLangOpts()); // Macros are simple: there's no declaration/definition distinction. // As a consequence, there's no need to look them up in the index either. @@ -408,10 +408,10 @@ std::vector<DocumentHighlight> findDocumentHighlights(ParsedAST &AST, Position Pos) { const SourceManager &SM = AST.getSourceManager(); // FIXME: show references to macro within file? - auto References = - findRefs(getDeclAtPosition( - AST, getBeginningOfIdentifier(AST, Pos, SM.getMainFileID())), - AST); + auto References = findRefs( + getDeclAtPosition(AST, getBeginningOfIdentifier( + Pos, SM, AST.getASTContext().getLangOpts())), + AST); // FIXME: we may get multiple DocumentHighlights with the same location and // different kinds, deduplicate them. @@ -876,7 +876,7 @@ llvm::Optional<HoverInfo> getHover(ParsedAST &AST, Position Pos, const SymbolIndex *Index) { llvm::Optional<HoverInfo> HI; SourceLocation SourceLocationBeg = getBeginningOfIdentifier( - AST, Pos, AST.getSourceManager().getMainFileID()); + Pos, AST.getSourceManager(), AST.getASTContext().getLangOpts()); if (auto M = locateMacroAt(SourceLocationBeg, AST.getPreprocessor())) { HI = getHoverContents(*M, AST); @@ -918,7 +918,8 @@ std::vector<Location> findReferences(ParsedAST &AST, Position Pos, elog("Failed to get a path for the main file, so no references"); return Results; } - auto Loc = getBeginningOfIdentifier(AST, Pos, SM.getMainFileID()); + auto Loc = + getBeginningOfIdentifier(Pos, SM, AST.getASTContext().getLangOpts()); // TODO: should we handle macros, too? auto Decls = getDeclAtPosition(AST, Loc); @@ -974,8 +975,8 @@ std::vector<Location> findReferences(ParsedAST &AST, Position Pos, std::vector<SymbolDetails> getSymbolInfo(ParsedAST &AST, Position Pos) { const SourceManager &SM = AST.getSourceManager(); - - auto Loc = getBeginningOfIdentifier(AST, Pos, SM.getMainFileID()); + auto Loc = + getBeginningOfIdentifier(Pos, SM, AST.getASTContext().getLangOpts()); std::vector<SymbolDetails> Results; @@ -1146,7 +1147,7 @@ static void fillSuperTypes(const CXXRecordDecl &CXXRD, ASTContext &ASTCtx, const CXXRecordDecl *findRecordTypeAt(ParsedAST &AST, Position Pos) { SourceLocation SourceLocationBeg = getBeginningOfIdentifier( - AST, Pos, AST.getSourceManager().getMainFileID()); + Pos, AST.getSourceManager(), AST.getASTContext().getLangOpts()); auto Decls = getDeclAtPosition(AST, SourceLocationBeg); if (Decls.empty()) return nullptr; diff --git a/clang-tools-extra/clangd/index/FileIndex.h b/clang-tools-extra/clangd/index/FileIndex.h index 125abc9a1ae..dcda26b1374 100644 --- a/clang-tools-extra/clangd/index/FileIndex.h +++ b/clang-tools-extra/clangd/index/FileIndex.h @@ -15,10 +15,10 @@ #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_FILEINDEX_H #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_FILEINDEX_H -#include "ClangdUnit.h" #include "Index.h" #include "MemIndex.h" #include "Merge.h" +#include "Path.h" #include "index/CanonicalIncludes.h" #include "index/Symbol.h" #include "clang/Lex/Preprocessor.h" @@ -26,6 +26,7 @@ namespace clang { namespace clangd { +class ParsedAST; /// Select between in-memory index implementations, which have tradeoffs. enum class IndexType { diff --git a/clang-tools-extra/clangd/refactor/Rename.cpp b/clang-tools-extra/clangd/refactor/Rename.cpp index 845aae17968..d62e1db52df 100644 --- a/clang-tools-extra/clangd/refactor/Rename.cpp +++ b/clang-tools-extra/clangd/refactor/Rename.cpp @@ -8,7 +8,9 @@ #include "refactor/Rename.h" #include "AST.h" +#include "ClangdUnit.h" #include "Logger.h" +#include "SourceCode.h" #include "index/SymbolCollector.h" #include "clang/Tooling/Refactoring/Rename/RenamingAction.h" #include "clang/Tooling/Refactoring/Rename/USRFinder.h" @@ -151,8 +153,8 @@ findOccurrencesWithinFile(ParsedAST &AST, const NamedDecl *RenameDecl) { llvm::Expected<tooling::Replacements> renameWithinFile(ParsedAST &AST, llvm::StringRef File, Position Pos, llvm::StringRef NewName, const SymbolIndex *Index) { - SourceLocation SourceLocationBeg = clangd::getBeginningOfIdentifier( - AST, Pos, AST.getSourceManager().getMainFileID()); + SourceLocation SourceLocationBeg = getBeginningOfIdentifier( + Pos, AST.getSourceManager(), AST.getASTContext().getLangOpts()); // FIXME: renaming macros is not supported yet, the macro-handling code should // be moved to rename tooling library. if (locateMacroAt(SourceLocationBeg, AST.getPreprocessor())) diff --git a/clang-tools-extra/clangd/refactor/Rename.h b/clang-tools-extra/clangd/refactor/Rename.h index e89ea22734c..63a1ffe3215 100644 --- a/clang-tools-extra/clangd/refactor/Rename.h +++ b/clang-tools-extra/clangd/refactor/Rename.h @@ -9,12 +9,14 @@ #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_RENAME_H #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_RENAME_H -#include "ClangdUnit.h" +#include "Protocol.h" #include "clang/Tooling/Core/Replacement.h" #include "llvm/Support/Error.h" namespace clang { namespace clangd { +class ParsedAST; +class SymbolIndex; /// Renames all occurrences of the symbol at \p Pos to \p NewName. /// Occurrences outside the current file are not modified. diff --git a/clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp b/clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp index 972f4efed51..8926d8849f6 100644 --- a/clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp +++ b/clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp @@ -30,45 +30,6 @@ using ::testing::ElementsAre; using ::testing::ElementsAreArray; using ::testing::AllOf; -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 (std::string Text : std::vector<std::string>{ - "int ^f^oo();", // inside identifier - "int ^foo();", // beginning of identifier - "int ^foo^();", // end of identifier - "int foo(^);", // non-identifier - "^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 - }) { - std::string WithPreamble = Preamble + Text; - Annotations TestCase(WithPreamble); - auto AST = TestTU::withCode(TestCase.code()).build(); - const auto &SourceMgr = AST.getSourceManager(); - SourceLocation Actual = getBeginningOfIdentifier( - AST, TestCase.points().back(), SourceMgr.getMainFileID()); - Position ActualPos = offsetToPosition( - TestCase.code(), - SourceMgr.getFileOffset(SourceMgr.getSpellingLoc(Actual))); - EXPECT_EQ(TestCase.points().front(), ActualPos) << Text; - } -} - MATCHER_P(DeclNamed, Name, "") { if (NamedDecl *ND = dyn_cast<NamedDecl>(arg)) if (ND->getName() == Name) diff --git a/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp b/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp index 8f9386e802a..d94d1c9f52b 100644 --- a/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp +++ b/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp @@ -312,6 +312,45 @@ TEST(SourceCodeTests, SourceLocationInMainFile) { } } +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 (std::string Text : std::vector<std::string>{ + "int ^f^oo();", // inside identifier + "int ^foo();", // beginning of identifier + "int ^foo^();", // end of identifier + "int foo(^);", // non-identifier + "^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 + }) { + std::string WithPreamble = Preamble + Text; + Annotations TestCase(WithPreamble); + auto AST = TestTU::withCode(TestCase.code()).build(); + const auto &SourceMgr = AST.getSourceManager(); + SourceLocation Actual = getBeginningOfIdentifier( + TestCase.points().back(), SourceMgr, AST.getASTContext().getLangOpts()); + Position ActualPos = offsetToPosition( + TestCase.code(), + SourceMgr.getFileOffset(SourceMgr.getSpellingLoc(Actual))); + EXPECT_EQ(TestCase.points().front(), ActualPos) << Text; + } +} + TEST(SourceCodeTests, CollectIdentifiers) { auto Style = format::getLLVMStyle(); auto IDs = collectIdentifiers(R"cpp( @@ -417,8 +456,8 @@ TEST(SourceCodeTests, GetMacros) { )cpp"); TestTU TU = TestTU::withCode(Code.code()); auto AST = TU.build(); - auto Loc = getBeginningOfIdentifier(AST, Code.point(), - AST.getSourceManager().getMainFileID()); + auto Loc = getBeginningOfIdentifier(Code.point(), AST.getSourceManager(), + AST.getASTContext().getLangOpts()); auto Result = locateMacroAt(Loc, AST.getPreprocessor()); ASSERT_TRUE(Result); EXPECT_THAT(*Result, MacroName("MACRO")); |