[clang-tools-extra] r324992 - [clangd] SymbolLocation only covers symbol name.
Haojian Wu via cfe-commits
cfe-commits at lists.llvm.org
Tue Feb 13 01:53:51 PST 2018
Author: hokein
Date: Tue Feb 13 01:53:50 2018
New Revision: 324992
URL: http://llvm.org/viewvc/llvm-project?rev=324992&view=rev
Log:
[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
Modified:
clang-tools-extra/trunk/clangd/index/Index.cpp
clang-tools-extra/trunk/clangd/index/Index.h
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
Modified: clang-tools-extra/trunk/clangd/index/Index.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.cpp?rev=324992&r1=324991&r2=324992&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/Index.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Index.cpp Tue Feb 13 01:53:50 2018
@@ -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)
Modified: clang-tools-extra/trunk/clangd/index/Index.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.h?rev=324992&r1=324991&r2=324992&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/Index.h (original)
+++ clang-tools-extra/trunk/clangd/index/Index.h Tue Feb 13 01:53:50 2018
@@ -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.
Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp?rev=324992&r1=324991&r2=324992&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp Tue Feb 13 01:53:50 2018
@@ -132,21 +132,14 @@ bool shouldFilterDecl(const NamedDecl *N
// 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, So
// 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::addDeclar
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(cons
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);
}
Modified: clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp?rev=324992&r1=324991&r2=324992&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp Tue Feb 13 01:53:50 2018
@@ -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, CollectSymbo
}
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, SymbolFormed
#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, SymbolFormed
#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, SymbolFormed
Annotations Header(R"(
#ifdef NAME
- $expansion[[class NAME {}]];
+ class $expansion[[NAME]] {};
#endif
)");
More information about the cfe-commits
mailing list