Index: clang-tools-extra/trunk/clangd/index/Index.h =================================================================== --- clang-tools-extra/trunk/clangd/index/Index.h +++ clang-tools-extra/trunk/clangd/index/Index.h @@ -11,6 +11,7 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_INDEX_H #include "clang/Index/IndexSymbol.h" +#include "clang/Lex/Lexer.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/DenseSet.h" #include "llvm/ADT/Hashing.h" @@ -25,11 +26,9 @@ struct SymbolLocation { // The URI of the source file where a symbol occurs. llvm::StringRef FileURI; - // The 0-based offset to the first character of the symbol from the beginning - // of the source file. + // The 0-based offsets of the symbol from the beginning of the source file, + // using half-open range, [StartOffset, EndOffset). unsigned StartOffset = 0; - // The 0-based offset to the last character of the symbol from the beginning - // of the source file. unsigned EndOffset = 0; operator bool() const { return !FileURI.empty(); } @@ -121,9 +120,10 @@ // The containing namespace. e.g. "" (global), "ns::" (top-level namespace). llvm::StringRef Scope; // The location of the symbol's definition, if one was found. - // This covers the whole definition (e.g. class body). + // This just covers the symbol name (e.g. without class/function body). SymbolLocation Definition; // The location of the preferred declaration of the symbol. + // This just covers the symbol name. // This may be the same as Definition. // // A C++ symbol may have multiple declarations, and we pick one to prefer. Index: clang-tools-extra/trunk/clangd/index/Index.cpp =================================================================== --- clang-tools-extra/trunk/clangd/index/Index.cpp +++ clang-tools-extra/trunk/clangd/index/Index.cpp @@ -19,7 +19,7 @@ raw_ostream &operator<<(raw_ostream &OS, const SymbolLocation &L) { if (!L) return OS << "(none)"; - return OS << L.FileURI << "[" << L.StartOffset << "-" << L.EndOffset << "]"; + return OS << L.FileURI << "[" << L.StartOffset << "-" << L.EndOffset << ")"; } SymbolID::SymbolID(StringRef USR) Index: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp =================================================================== --- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp +++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp @@ -132,21 +132,14 @@ // For symbols defined inside macros: // * use expansion location, if the symbol is formed via macro concatenation. // * use spelling location, otherwise. -// -// FIXME: EndOffset is inclusive (closed range), and should be exclusive. -// FIXME: Because the underlying ranges are token ranges, this code chops the -// last token in half if it contains multiple characters. -// FIXME: We probably want to get just the location of the symbol name, not -// the whole e.g. class. llvm::Optional getSymbolLocation(const NamedDecl &D, SourceManager &SM, const SymbolCollector::Options &Opts, + const clang::LangOptions& LangOpts, std::string &FileURIStorage) { - SourceLocation Loc = D.getLocation(); - SourceLocation StartLoc = SM.getSpellingLoc(D.getLocStart()); - SourceLocation EndLoc = SM.getSpellingLoc(D.getLocEnd()); - if (Loc.isMacroID()) { - std::string PrintLoc = SM.getSpellingLoc(Loc).printToString(SM); + SourceLocation SpellingLoc = SM.getSpellingLoc(D.getLocation()); + if (D.getLocation().isMacroID()) { + std::string PrintLoc = SpellingLoc.printToString(SM); if (llvm::StringRef(PrintLoc).startswith("")) { // We use the expansion location for the following symbols, as spelling @@ -155,19 +148,19 @@ // be "" // * symbols controlled and defined by a compile command-line option // `-DName=foo`, the spelling location will be "". - StartLoc = SM.getExpansionRange(D.getLocStart()).first; - EndLoc = SM.getExpansionRange(D.getLocEnd()).second; + SpellingLoc = SM.getExpansionRange(D.getLocation()).first; } } - auto U = toURI(SM, SM.getFilename(StartLoc), Opts); + auto U = toURI(SM, SM.getFilename(SpellingLoc), Opts); if (!U) return llvm::None; FileURIStorage = std::move(*U); SymbolLocation Result; Result.FileURI = FileURIStorage; - Result.StartOffset = SM.getFileOffset(StartLoc); - Result.EndOffset = SM.getFileOffset(EndLoc); + Result.StartOffset = SM.getFileOffset(SpellingLoc); + Result.EndOffset = Result.StartOffset + clang::Lexer::MeasureTokenLength( + SpellingLoc, SM, LangOpts); return std::move(Result); } @@ -235,7 +228,8 @@ std::string FileURI; // FIXME: we may want a different "canonical" heuristic than clang chooses. // Clang seems to choose the first, which may not have the most information. - if (auto DeclLoc = getSymbolLocation(ND, SM, Opts, FileURI)) + if (auto DeclLoc = + getSymbolLocation(ND, SM, Opts, ASTCtx->getLangOpts(), FileURI)) S.CanonicalDeclaration = *DeclLoc; // Add completion info. @@ -281,7 +275,7 @@ Symbol S = DeclSym; std::string FileURI; if (auto DefLoc = getSymbolLocation(ND, ND.getASTContext().getSourceManager(), - Opts, FileURI)) + Opts, ASTCtx->getLangOpts(), FileURI)) S.Definition = *DefLoc; Symbols.insert(S); } Index: clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp =================================================================== --- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp +++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp @@ -48,15 +48,12 @@ MATCHER_P(QName, Name, "") { return (arg.Scope + arg.Name).str() == Name; } MATCHER_P(DeclURI, P, "") { return arg.CanonicalDeclaration.FileURI == P; } MATCHER_P(DeclRange, Offsets, "") { - // Offset range in SymbolLocation is [start, end] while in Clangd is [start, - // end). - // FIXME: SymbolLocation should be [start, end). return arg.CanonicalDeclaration.StartOffset == Offsets.first && - arg.CanonicalDeclaration.EndOffset == Offsets.second - 1; + arg.CanonicalDeclaration.EndOffset == Offsets.second; } MATCHER_P(DefRange, Offsets, "") { return arg.Definition.StartOffset == Offsets.first && - arg.Definition.EndOffset == Offsets.second - 1; + arg.Definition.EndOffset == Offsets.second; } namespace clang { @@ -177,25 +174,23 @@ } TEST_F(SymbolCollectorTest, Locations) { - // FIXME: the behavior here is bad: chopping tokens, including more than the - // ident range, using half-open ranges. See fixmes in getSymbolLocation(). CollectorOpts.IndexMainFiles = true; Annotations Header(R"cpp( // Declared in header, defined in main. - $xdecl[[extern int X]]; - $clsdecl[[class C]]ls; - $printdecl[[void print()]]; + extern int $xdecl[[X]]; + class $clsdecl[[Cls]]; + void $printdecl[[print]](); // Declared in header, defined nowhere. - $zdecl[[extern int Z]]; + extern int $zdecl[[Z]]; )cpp"); Annotations Main(R"cpp( - $xdef[[int X = 4]]2; - $clsdef[[class Cls {}]]; - $printdef[[void print() {}]] + int $xdef[[X]] = 42; + class $clsdef[[Cls]] {}; + void $printdef[[print]]() {} // Declared/defined in main only. - $y[[int Y]]; + int $y[[Y]]; )cpp"); runSymbolCollector(Header.code(), Main.code()); EXPECT_THAT( @@ -304,10 +299,10 @@ #define FF(name) \ class name##_Test {}; - $expansion[[FF(abc)]]; + $expansion[[FF]](abc); #define FF2() \ - $spelling[[class Test {}]]; + class $spelling[[Test]] {}; FF2(); )"); @@ -329,10 +324,10 @@ #define FF(name) \ class name##_Test {}; - $expansion[[FF(abc)]]; + $expansion[[FF]](abc); #define FF2() \ - $spelling[[class Test {}]]; + class $spelling[[Test]] {}; FF2(); )"); @@ -351,7 +346,7 @@ Annotations Header(R"( #ifdef NAME - $expansion[[class NAME {}]]; + class $expansion[[NAME]] {}; #endif )");