[clangd] SymbolLocation only covers symbol name.
Summary: * Change the offset range to half-open, [start, end). * Fix a few fixmes. Reviewers: sammccall Subscribers: klimek, ilya-biryukov, jkorous-apple, ioeric, cfe-commits Differential Revision: https://reviews.llvm.org/D43182 llvm-svn: 324992
This commit is contained in:
parent
3bd0a15867
commit
dc02a3d943
|
@ -19,7 +19,7 @@ using namespace llvm;
|
|||
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)
|
||||
|
|
|
@ -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 @@ namespace clangd {
|
|||
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 @@ struct Symbol {
|
|||
// 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.
|
||||
|
|
|
@ -132,21 +132,14 @@ bool shouldFilterDecl(const NamedDecl *ND, ASTContext *ASTCtx,
|
|||
// 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
|
||||
|
@ -155,19 +148,19 @@ getSymbolLocation(const NamedDecl &D, SourceManager &SM,
|
|||
// 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 @@ const Symbol *SymbolCollector::addDeclaration(const NamedDecl &ND,
|
|||
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 @@ void SymbolCollector::addDefinition(const NamedDecl &ND,
|
|||
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);
|
||||
}
|
||||
|
|
|
@ -48,15 +48,12 @@ MATCHER_P(Snippet, S, "") {
|
|||
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, CollectSymbols) {
|
|||
}
|
||||
|
||||
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 @@ TEST_F(SymbolCollectorTest, SymbolFormedFromMacro) {
|
|||
#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 @@ TEST_F(SymbolCollectorTest, SymbolFormedFromMacroInMainFile) {
|
|||
#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 @@ TEST_F(SymbolCollectorTest, SymbolFormedByCLI) {
|
|||
|
||||
Annotations Header(R"(
|
||||
#ifdef NAME
|
||||
$expansion[[class NAME {}]];
|
||||
class $expansion[[NAME]] {};
|
||||
#endif
|
||||
)");
|
||||
|
||||
|
|
Loading…
Reference in New Issue