hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: ioeric, jkorous-apple, ilya-biryukov, klimek.
- Change the offset range to half-open, [start, end).
- Fix a few fixmes.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43182
Files:
clangd/index/Index.cpp
clangd/index/Index.h
clangd/index/SymbolCollector.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===================================================================
--- unittests/clangd/SymbolCollectorTests.cpp
+++ 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
)");
Index: clangd/index/SymbolCollector.cpp
===================================================================
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -132,42 +132,35 @@
// 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<SymbolLocation>
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("<scratch") ||
llvm::StringRef(PrintLoc).startswith("<command line>")) {
// We use the expansion location for the following symbols, as spelling
// locations of these symbols are not interesting to us:
// * symbols formed via macro concatenation, the spelling location will
// be "<scratch space>"
// * symbols controlled and defined by a compile command-line option
// `-DName=foo`, the spelling location will be "<command line>".
- 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: clangd/index/Index.h
===================================================================
--- clangd/index/Index.h
+++ 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.
+ // The location of the preferred declaration of the symbol, 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: clangd/index/Index.cpp
===================================================================
--- clangd/index/Index.cpp
+++ 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)
@@ -37,7 +37,8 @@
}
raw_ostream &operator<<(raw_ostream &OS, const Symbol &S) {
- return OS << S.Scope << S.Name;
+ return OS << S.Scope << S.Name << " " << S.CanonicalDeclaration << ", "
+ << S.Definition << "\n";
}
SymbolSlab::const_iterator SymbolSlab::find(const SymbolID &ID) const {
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
