Skip to content

Commit 19cefc2

Browse files
sam-mccallsam-mccall
committed
[clangd] Move getBeginningOfIdentifier from ClangdUnit to SourceCode. Drop dependencies on ClangdUnit from some headers. NFC
llvm-svn: 370768
1 parent 3b18b05 commit 19cefc2

15 files changed

+105
-99
lines changed

‎clang-tools-extra/clangd/ClangdServer.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,7 @@ void ClangdServer::prepareRename(PathRef File, Position Pos,
315315
return CB(Changes.takeError());
316316
}
317317
SourceLocation Loc = getBeginningOfIdentifier(
318-
AST, Pos, AST.getSourceManager().getMainFileID());
318+
Pos, AST.getSourceManager(), AST.getASTContext().getLangOpts());
319319
if (auto Range = getTokenRange(AST.getSourceManager(),
320320
AST.getASTContext().getLangOpts(), Loc))
321321
return CB(*Range);

‎clang-tools-extra/clangd/ClangdServer.h

-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111

1212
#include "../clang-tidy/ClangTidyOptions.h"
1313
#include "Cancellation.h"
14-
#include "ClangdUnit.h"
1514
#include "CodeComplete.h"
1615
#include "FSProvider.h"
1716
#include "FormattedString.h"

‎clang-tools-extra/clangd/ClangdUnit.cpp

-33
Original file line numberDiff line numberDiff line change
@@ -710,39 +710,6 @@ buildAST(PathRef FileName, std::unique_ptr<CompilerInvocation> Invocation,
710710
std::move(VFS), Inputs.Index, Inputs.Opts);
711711
}
712712

713-
SourceLocation getBeginningOfIdentifier(const ParsedAST &Unit,
714-
const Position &Pos, const FileID FID) {
715-
const ASTContext &AST = Unit.getASTContext();
716-
const SourceManager &SourceMgr = AST.getSourceManager();
717-
auto Offset = positionToOffset(SourceMgr.getBufferData(FID), Pos);
718-
if (!Offset) {
719-
log("getBeginningOfIdentifier: {0}", Offset.takeError());
720-
return SourceLocation();
721-
}
722-
723-
// GetBeginningOfToken(pos) is almost what we want, but does the wrong thing
724-
// if the cursor is at the end of the identifier.
725-
// Instead, we lex at GetBeginningOfToken(pos - 1). The cases are:
726-
// 1) at the beginning of an identifier, we'll be looking at something
727-
// that isn't an identifier.
728-
// 2) at the middle or end of an identifier, we get the identifier.
729-
// 3) anywhere outside an identifier, we'll get some non-identifier thing.
730-
// We can't actually distinguish cases 1 and 3, but returning the original
731-
// location is correct for both!
732-
SourceLocation InputLoc = SourceMgr.getComposedLoc(FID, *Offset);
733-
if (*Offset == 0) // Case 1 or 3.
734-
return SourceMgr.getMacroArgExpandedLocation(InputLoc);
735-
SourceLocation Before = SourceMgr.getComposedLoc(FID, *Offset - 1);
736-
737-
Before = Lexer::GetBeginningOfToken(Before, SourceMgr, AST.getLangOpts());
738-
Token Tok;
739-
if (Before.isValid() &&
740-
!Lexer::getRawToken(Before, Tok, SourceMgr, AST.getLangOpts(), false) &&
741-
Tok.is(tok::raw_identifier))
742-
return SourceMgr.getMacroArgExpandedLocation(Before); // Case 2.
743-
return SourceMgr.getMacroArgExpandedLocation(InputLoc); // Case 1 or 3.
744-
}
745-
746713
} // namespace clangd
747714
namespace tidy {
748715
// Force the linker to link in Clang-tidy modules.

‎clang-tools-extra/clangd/ClangdUnit.h

-5
Original file line numberDiff line numberDiff line change
@@ -187,11 +187,6 @@ buildAST(PathRef FileName, std::unique_ptr<CompilerInvocation> Invocation,
187187
const ParseInputs &Inputs,
188188
std::shared_ptr<const PreambleData> Preamble);
189189

190-
/// Get the beginning SourceLocation at a specified \p Pos.
191-
/// May be invalid if Pos is, or if there's no identifier.
192-
SourceLocation getBeginningOfIdentifier(const ParsedAST &Unit,
193-
const Position &Pos, const FileID FID);
194-
195190
/// For testing/debugging purposes. Note that this method deserializes all
196191
/// unserialized Decls, so use with care.
197192
void dumpAST(ParsedAST &AST, llvm::raw_ostream &OS);

‎clang-tools-extra/clangd/CodeComplete.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_CODECOMPLETE_H
1616
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_CODECOMPLETE_H
1717

18-
#include "ClangdUnit.h"
1918
#include "Headers.h"
2019
#include "Logger.h"
2120
#include "Path.h"
@@ -36,6 +35,7 @@
3635
namespace clang {
3736
class NamedDecl;
3837
namespace clangd {
38+
struct PreambleData;
3939

4040
struct CodeCompleteOptions {
4141
/// Returns options that can be passed to clang's completion engine.

‎clang-tools-extra/clangd/Selection.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "Selection.h"
10-
#include "ClangdUnit.h"
1110
#include "Logger.h"
1211
#include "SourceCode.h"
1312
#include "clang/AST/ASTTypeTraits.h"
@@ -17,6 +16,7 @@
1716
#include "clang/Basic/SourceLocation.h"
1817
#include "clang/Basic/SourceManager.h"
1918
#include "clang/Basic/TokenKinds.h"
19+
#include "clang/Lex/Lexer.h"
2020
#include "clang/Tooling/Syntax/Tokens.h"
2121
#include "llvm/ADT/STLExtras.h"
2222
#include "llvm/Support/raw_ostream.h"

‎clang-tools-extra/clangd/Selection.h

-1
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@
4040

4141
namespace clang {
4242
namespace clangd {
43-
class ParsedAST;
4443

4544
// A selection can partially or completely cover several AST nodes.
4645
// The SelectionTree contains nodes that are covered, and their parents.

‎clang-tools-extra/clangd/SourceCode.cpp

+33
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,39 @@ llvm::Optional<Range> getTokenRange(const SourceManager &SM,
229229
return halfOpenToRange(SM, CharSourceRange::getCharRange(TokLoc, End));
230230
}
231231

232+
SourceLocation getBeginningOfIdentifier(const Position &Pos,
233+
const SourceManager &SM,
234+
const LangOptions &LangOpts) {
235+
FileID FID = SM.getMainFileID();
236+
auto Offset = positionToOffset(SM.getBufferData(FID), Pos);
237+
if (!Offset) {
238+
log("getBeginningOfIdentifier: {0}", Offset.takeError());
239+
return SourceLocation();
240+
}
241+
242+
// GetBeginningOfToken(pos) is almost what we want, but does the wrong thing
243+
// if the cursor is at the end of the identifier.
244+
// Instead, we lex at GetBeginningOfToken(pos - 1). The cases are:
245+
// 1) at the beginning of an identifier, we'll be looking at something
246+
// that isn't an identifier.
247+
// 2) at the middle or end of an identifier, we get the identifier.
248+
// 3) anywhere outside an identifier, we'll get some non-identifier thing.
249+
// We can't actually distinguish cases 1 and 3, but returning the original
250+
// location is correct for both!
251+
SourceLocation InputLoc = SM.getComposedLoc(FID, *Offset);
252+
if (*Offset == 0) // Case 1 or 3.
253+
return SM.getMacroArgExpandedLocation(InputLoc);
254+
SourceLocation Before = SM.getComposedLoc(FID, *Offset - 1);
255+
256+
Before = Lexer::GetBeginningOfToken(Before, SM, LangOpts);
257+
Token Tok;
258+
if (Before.isValid() &&
259+
!Lexer::getRawToken(Before, Tok, SM, LangOpts, false) &&
260+
Tok.is(tok::raw_identifier))
261+
return SM.getMacroArgExpandedLocation(Before); // Case 2.
262+
return SM.getMacroArgExpandedLocation(InputLoc); // Case 1 or 3.
263+
}
264+
232265
bool isValidFileRange(const SourceManager &Mgr, SourceRange R) {
233266
if (!R.getBegin().isValid() || !R.getEnd().isValid())
234267
return false;

‎clang-tools-extra/clangd/SourceCode.h

+7
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,13 @@ llvm::Optional<Range> getTokenRange(const SourceManager &SM,
7575
llvm::Expected<SourceLocation> sourceLocationInMainFile(const SourceManager &SM,
7676
Position P);
7777

78+
/// Get the beginning SourceLocation at a specified \p Pos in the main file.
79+
/// May be invalid if Pos is, or if there's no identifier.
80+
/// FIXME: this returns the macro-expansion location, but it shouldn't.
81+
SourceLocation getBeginningOfIdentifier(const Position &Pos,
82+
const SourceManager &SM,
83+
const LangOptions &LangOpts);
84+
7885
/// Returns true iff \p Loc is inside the main file. This function handles
7986
/// file & macro locations. For macro locations, returns iff the macro is being
8087
/// expanded inside the main file.

‎clang-tools-extra/clangd/XRefs.cpp

+12-11
Original file line numberDiff line numberDiff line change
@@ -254,8 +254,8 @@ std::vector<LocatedSymbol> locateSymbolAt(ParsedAST &AST, Position Pos,
254254
}
255255
}
256256

257-
SourceLocation SourceLocationBeg =
258-
getBeginningOfIdentifier(AST, Pos, SM.getMainFileID());
257+
SourceLocation SourceLocationBeg = getBeginningOfIdentifier(
258+
Pos, AST.getSourceManager(), AST.getASTContext().getLangOpts());
259259

260260
// Macros are simple: there's no declaration/definition distinction.
261261
// 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,
408408
Position Pos) {
409409
const SourceManager &SM = AST.getSourceManager();
410410
// FIXME: show references to macro within file?
411-
auto References =
412-
findRefs(getDeclAtPosition(
413-
AST, getBeginningOfIdentifier(AST, Pos, SM.getMainFileID())),
414-
AST);
411+
auto References = findRefs(
412+
getDeclAtPosition(AST, getBeginningOfIdentifier(
413+
Pos, SM, AST.getASTContext().getLangOpts())),
414+
AST);
415415

416416
// FIXME: we may get multiple DocumentHighlights with the same location and
417417
// different kinds, deduplicate them.
@@ -876,7 +876,7 @@ llvm::Optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
876876
const SymbolIndex *Index) {
877877
llvm::Optional<HoverInfo> HI;
878878
SourceLocation SourceLocationBeg = getBeginningOfIdentifier(
879-
AST, Pos, AST.getSourceManager().getMainFileID());
879+
Pos, AST.getSourceManager(), AST.getASTContext().getLangOpts());
880880

881881
if (auto M = locateMacroAt(SourceLocationBeg, AST.getPreprocessor())) {
882882
HI = getHoverContents(*M, AST);
@@ -918,7 +918,8 @@ std::vector<Location> findReferences(ParsedAST &AST, Position Pos,
918918
elog("Failed to get a path for the main file, so no references");
919919
return Results;
920920
}
921-
auto Loc = getBeginningOfIdentifier(AST, Pos, SM.getMainFileID());
921+
auto Loc =
922+
getBeginningOfIdentifier(Pos, SM, AST.getASTContext().getLangOpts());
922923
// TODO: should we handle macros, too?
923924
auto Decls = getDeclAtPosition(AST, Loc);
924925

@@ -974,8 +975,8 @@ std::vector<Location> findReferences(ParsedAST &AST, Position Pos,
974975

975976
std::vector<SymbolDetails> getSymbolInfo(ParsedAST &AST, Position Pos) {
976977
const SourceManager &SM = AST.getSourceManager();
977-
978-
auto Loc = getBeginningOfIdentifier(AST, Pos, SM.getMainFileID());
978+
auto Loc =
979+
getBeginningOfIdentifier(Pos, SM, AST.getASTContext().getLangOpts());
979980

980981
std::vector<SymbolDetails> Results;
981982

@@ -1146,7 +1147,7 @@ static void fillSuperTypes(const CXXRecordDecl &CXXRD, ASTContext &ASTCtx,
11461147

11471148
const CXXRecordDecl *findRecordTypeAt(ParsedAST &AST, Position Pos) {
11481149
SourceLocation SourceLocationBeg = getBeginningOfIdentifier(
1149-
AST, Pos, AST.getSourceManager().getMainFileID());
1150+
Pos, AST.getSourceManager(), AST.getASTContext().getLangOpts());
11501151
auto Decls = getDeclAtPosition(AST, SourceLocationBeg);
11511152
if (Decls.empty())
11521153
return nullptr;

‎clang-tools-extra/clangd/index/FileIndex.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -15,17 +15,18 @@
1515
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_FILEINDEX_H
1616
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_FILEINDEX_H
1717

18-
#include "ClangdUnit.h"
1918
#include "Index.h"
2019
#include "MemIndex.h"
2120
#include "Merge.h"
21+
#include "Path.h"
2222
#include "index/CanonicalIncludes.h"
2323
#include "index/Symbol.h"
2424
#include "clang/Lex/Preprocessor.h"
2525
#include <memory>
2626

2727
namespace clang {
2828
namespace clangd {
29+
class ParsedAST;
2930

3031
/// Select between in-memory index implementations, which have tradeoffs.
3132
enum class IndexType {

‎clang-tools-extra/clangd/refactor/Rename.cpp

+4-2
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@
88

99
#include "refactor/Rename.h"
1010
#include "AST.h"
11+
#include "ClangdUnit.h"
1112
#include "Logger.h"
13+
#include "SourceCode.h"
1214
#include "index/SymbolCollector.h"
1315
#include "clang/Tooling/Refactoring/Rename/RenamingAction.h"
1416
#include "clang/Tooling/Refactoring/Rename/USRFinder.h"
@@ -151,8 +153,8 @@ findOccurrencesWithinFile(ParsedAST &AST, const NamedDecl *RenameDecl) {
151153
llvm::Expected<tooling::Replacements>
152154
renameWithinFile(ParsedAST &AST, llvm::StringRef File, Position Pos,
153155
llvm::StringRef NewName, const SymbolIndex *Index) {
154-
SourceLocation SourceLocationBeg = clangd::getBeginningOfIdentifier(
155-
AST, Pos, AST.getSourceManager().getMainFileID());
156+
SourceLocation SourceLocationBeg = getBeginningOfIdentifier(
157+
Pos, AST.getSourceManager(), AST.getASTContext().getLangOpts());
156158
// FIXME: renaming macros is not supported yet, the macro-handling code should
157159
// be moved to rename tooling library.
158160
if (locateMacroAt(SourceLocationBeg, AST.getPreprocessor()))

‎clang-tools-extra/clangd/refactor/Rename.h

+3-1
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,14 @@
99
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_RENAME_H
1010
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_RENAME_H
1111

12-
#include "ClangdUnit.h"
12+
#include "Protocol.h"
1313
#include "clang/Tooling/Core/Replacement.h"
1414
#include "llvm/Support/Error.h"
1515

1616
namespace clang {
1717
namespace clangd {
18+
class ParsedAST;
19+
class SymbolIndex;
1820

1921
/// Renames all occurrences of the symbol at \p Pos to \p NewName.
2022
/// Occurrences outside the current file are not modified.

‎clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp

-39
Original file line numberDiff line numberDiff line change
@@ -30,45 +30,6 @@ using ::testing::ElementsAre;
3030
using ::testing::ElementsAreArray;
3131
using ::testing::AllOf;
3232

33-
TEST(ClangdUnitTest, GetBeginningOfIdentifier) {
34-
std::string Preamble = R"cpp(
35-
struct Bar { int func(); };
36-
#define MACRO(X) void f() { X; }
37-
Bar* bar;
38-
)cpp";
39-
// First ^ is the expected beginning, last is the search position.
40-
for (std::string Text : std::vector<std::string>{
41-
"int ^f^oo();", // inside identifier
42-
"int ^foo();", // beginning of identifier
43-
"int ^foo^();", // end of identifier
44-
"int foo(^);", // non-identifier
45-
"^int foo();", // beginning of file (can't back up)
46-
"int ^f0^0();", // after a digit (lexing at N-1 is wrong)
47-
"int ^λλ^λ();", // UTF-8 handled properly when backing up
48-
49-
// identifier in macro arg
50-
"MACRO(bar->^func())", // beginning of identifier
51-
"MACRO(bar->^fun^c())", // inside identifier
52-
"MACRO(bar->^func^())", // end of identifier
53-
"MACRO(^bar->func())", // begin identifier
54-
"MACRO(^bar^->func())", // end identifier
55-
"^MACRO(bar->func())", // beginning of macro name
56-
"^MAC^RO(bar->func())", // inside macro name
57-
"^MACRO^(bar->func())", // end of macro name
58-
}) {
59-
std::string WithPreamble = Preamble + Text;
60-
Annotations TestCase(WithPreamble);
61-
auto AST = TestTU::withCode(TestCase.code()).build();
62-
const auto &SourceMgr = AST.getSourceManager();
63-
SourceLocation Actual = getBeginningOfIdentifier(
64-
AST, TestCase.points().back(), SourceMgr.getMainFileID());
65-
Position ActualPos = offsetToPosition(
66-
TestCase.code(),
67-
SourceMgr.getFileOffset(SourceMgr.getSpellingLoc(Actual)));
68-
EXPECT_EQ(TestCase.points().front(), ActualPos) << Text;
69-
}
70-
}
71-
7233
MATCHER_P(DeclNamed, Name, "") {
7334
if (NamedDecl *ND = dyn_cast<NamedDecl>(arg))
7435
if (ND->getName() == Name)

‎clang-tools-extra/clangd/unittests/SourceCodeTests.cpp

+41-2
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,45 @@ TEST(SourceCodeTests, SourceLocationInMainFile) {
312312
}
313313
}
314314

315+
TEST(ClangdUnitTest, GetBeginningOfIdentifier) {
316+
std::string Preamble = R"cpp(
317+
struct Bar { int func(); };
318+
#define MACRO(X) void f() { X; }
319+
Bar* bar;
320+
)cpp";
321+
// First ^ is the expected beginning, last is the search position.
322+
for (std::string Text : std::vector<std::string>{
323+
"int ^f^oo();", // inside identifier
324+
"int ^foo();", // beginning of identifier
325+
"int ^foo^();", // end of identifier
326+
"int foo(^);", // non-identifier
327+
"^int foo();", // beginning of file (can't back up)
328+
"int ^f0^0();", // after a digit (lexing at N-1 is wrong)
329+
"int ^λλ^λ();", // UTF-8 handled properly when backing up
330+
331+
// identifier in macro arg
332+
"MACRO(bar->^func())", // beginning of identifier
333+
"MACRO(bar->^fun^c())", // inside identifier
334+
"MACRO(bar->^func^())", // end of identifier
335+
"MACRO(^bar->func())", // begin identifier
336+
"MACRO(^bar^->func())", // end identifier
337+
"^MACRO(bar->func())", // beginning of macro name
338+
"^MAC^RO(bar->func())", // inside macro name
339+
"^MACRO^(bar->func())", // end of macro name
340+
}) {
341+
std::string WithPreamble = Preamble + Text;
342+
Annotations TestCase(WithPreamble);
343+
auto AST = TestTU::withCode(TestCase.code()).build();
344+
const auto &SourceMgr = AST.getSourceManager();
345+
SourceLocation Actual = getBeginningOfIdentifier(
346+
TestCase.points().back(), SourceMgr, AST.getASTContext().getLangOpts());
347+
Position ActualPos = offsetToPosition(
348+
TestCase.code(),
349+
SourceMgr.getFileOffset(SourceMgr.getSpellingLoc(Actual)));
350+
EXPECT_EQ(TestCase.points().front(), ActualPos) << Text;
351+
}
352+
}
353+
315354
TEST(SourceCodeTests, CollectIdentifiers) {
316355
auto Style = format::getLLVMStyle();
317356
auto IDs = collectIdentifiers(R"cpp(
@@ -417,8 +456,8 @@ TEST(SourceCodeTests, GetMacros) {
417456
)cpp");
418457
TestTU TU = TestTU::withCode(Code.code());
419458
auto AST = TU.build();
420-
auto Loc = getBeginningOfIdentifier(AST, Code.point(),
421-
AST.getSourceManager().getMainFileID());
459+
auto Loc = getBeginningOfIdentifier(Code.point(), AST.getSourceManager(),
460+
AST.getASTContext().getLangOpts());
422461
auto Result = locateMacroAt(Loc, AST.getPreprocessor());
423462
ASSERT_TRUE(Result);
424463
EXPECT_THAT(*Result, MacroName("MACRO"));

0 commit comments

Comments
 (0)