[clangd] Store full decl/def range with symbol locations

clang-tools-extra/clangd/CodeComplete.cpp

@@ -418,7 +418,7 @@ struct CodeCompletionBuilder {
     auto Inserted = [&](llvm::StringRef Header)
         -> llvm::Expected<std::pair<std::string, bool>> {
       auto ResolvedDeclaring =
-          URI::resolve(C.IndexResult->CanonicalDeclaration.FileURI, FileName);
+          URI::resolve(C.IndexResult->CanonicalDeclaration.fileURI(), FileName);
       if (!ResolvedDeclaring)
         return ResolvedDeclaring.takeError();
       auto ResolvedInserted = toHeaderFile(Header, FileName);
@@ -451,7 +451,7 @@ struct CodeCompletionBuilder {
       } else
         log("Failed to generate include insertion edits for adding header "
             "(FileURI='{0}', IncludeHeader='{1}') into {2}: {3}",
-            C.IndexResult->CanonicalDeclaration.FileURI, Inc.Header, FileName,
+            C.IndexResult->CanonicalDeclaration.fileURI(), Inc.Header, FileName,
             ToInclude.takeError());
     }
     // Prefer includes that do not need edits (i.e. already exist).

clang-tools-extra/clangd/FindSymbols.cpp

@@ -54,27 +54,41 @@ bool approximateScopeMatch(llvm::StringRef Scope, llvm::StringRef Query) {
   return Query.empty();
 }
 
+Range indexToLSPRange(const SymbolPosition &SrcStart,
+                      const SymbolPosition &SrcEnd) {
+  Position Start, End;
+  Start.line = SrcStart.line();
+  Start.character = SrcStart.column();
+  End.line = SrcEnd.line();
+  End.character = SrcEnd.column();
+  return {Start, End};
+}
+
 } // namespace
 
-llvm::Expected<Location> indexToLSPLocation(const SymbolLocation &Loc,
+llvm::Expected<Location> indexToLSPLocation(const SymbolNameLocation &Loc,
                                             llvm::StringRef TUPath) {
   auto Path = URI::resolve(Loc.FileURI, TUPath);
   if (!Path)
     return error("Could not resolve path for file '{0}': {1}", Loc.FileURI,
                  Path.takeError());
   Location L;
   L.uri = URIForFile::canonicalize(*Path, TUPath);
-  Position Start, End;
-  Start.line = Loc.Start.line();
-  Start.character = Loc.Start.column();
-  End.line = Loc.End.line();
-  End.character = Loc.End.column();
-  L.range = {Start, End};
+  L.range = indexToLSPRange(Loc.Start, Loc.End);
   return L;
 }
 
-llvm::Expected<Location> symbolToLocation(const Symbol &Sym,
-                                          llvm::StringRef TUPath) {
+llvm::Expected<std::pair<Location, Range>>
+indexToLSPLocation(const SymbolDeclDefLocation &Loc, StringRef TUPath) {
+  auto L = indexToLSPLocation(Loc.NameLocation, TUPath);
+  if (!L)
+    return L.takeError();
+  return std::make_pair(L.get(),
+                        indexToLSPRange(Loc.DeclDefStart, Loc.DeclDefEnd));
+}
+
+llvm::Expected<std::pair<Location, Range>>
+symbolToLocation(const Symbol &Sym, llvm::StringRef TUPath) {
   // Prefer the definition over e.g. a function declaration in a header
   return indexToLSPLocation(
       Sym.Definition ? Sym.Definition : Sym.CanonicalDeclaration, TUPath);
@@ -152,7 +166,7 @@ getWorkspaceSymbols(llvm::StringRef Query, int Limit,
     SymbolInformation Info;
     Info.name = (Sym.Name + Sym.TemplateSpecializationArgs).str();
     Info.kind = indexSymbolKindToSymbolKind(Sym.SymInfo.Kind);
-    Info.location = *Loc;
+    Info.location = Loc->first;
     Scope.consume_back("::");
     Info.containerName = Scope.str();
 

clang-tools-extra/clangd/FindSymbols.h

@@ -22,12 +22,14 @@ class ParsedAST;
 class SymbolIndex;
 
 /// Helper function for deriving an LSP Location from an index SymbolLocation.
-llvm::Expected<Location> indexToLSPLocation(const SymbolLocation &Loc,
+llvm::Expected<Location> indexToLSPLocation(const SymbolNameLocation &Loc,
                                             llvm::StringRef TUPath);
+llvm::Expected<std::pair<Location, Range>>
+indexToLSPLocation(const SymbolDeclDefLocation &Loc, llvm::StringRef TUPath);
 
 /// Helper function for deriving an LSP Location for a Symbol.
-llvm::Expected<Location> symbolToLocation(const Symbol &Sym,
-                                          llvm::StringRef TUPath);
+llvm::Expected<std::pair<Location, Range>>
+symbolToLocation(const Symbol &Sym, llvm::StringRef TUPath);
 
 /// Searches for the symbols matching \p Query. The syntax of \p Query can be
 /// the non-qualified name or fully qualified of a symbol. For example,

clang-tools-extra/clangd/HeaderSourceSwitch.cpp

@@ -95,9 +95,9 @@ std::optional<Path> getCorrespondingHeaderOrSource(PathRef OriginalFile,
   bool IsHeader = isHeaderFile(OriginalFile, AST.getLangOpts());
   Index->lookup(Request, [&](const Symbol &Sym) {
     if (IsHeader)
-      AwardTarget(Sym.Definition.FileURI);
+      AwardTarget(Sym.Definition.fileURI());
     else
-      AwardTarget(Sym.CanonicalDeclaration.FileURI);
+      AwardTarget(Sym.CanonicalDeclaration.fileURI());
   });
   // FIXME: our index doesn't have any interesting information (this could be
   // that the background-index is not finished), we should use the decl/def

clang-tools-extra/clangd/IncludeFixer.cpp

@@ -289,7 +289,7 @@ std::vector<Fix> IncludeFixer::fixIncompleteType(const Type &T) const {
   if (!Syms.empty()) {
     auto &Matched = *Syms.begin();
     if (!Matched.IncludeHeaders.empty() && Matched.Definition &&
-        Matched.CanonicalDeclaration.FileURI == Matched.Definition.FileURI)
+        Matched.CanonicalDeclaration.fileURI() == Matched.Definition.fileURI())
       Fixes = fixesForSymbols(Syms);
   }
   return Fixes;
@@ -299,7 +299,7 @@ std::vector<Fix> IncludeFixer::fixesForSymbols(const SymbolSlab &Syms) const {
   auto Inserted = [&](const Symbol &Sym, llvm::StringRef Header)
       -> llvm::Expected<std::pair<std::string, bool>> {
     auto ResolvedDeclaring =
-        URI::resolve(Sym.CanonicalDeclaration.FileURI, File);
+        URI::resolve(Sym.CanonicalDeclaration.fileURI(), File);
     if (!ResolvedDeclaring)
       return ResolvedDeclaring.takeError();
     auto ResolvedInserted = toHeaderFile(Header, File);
@@ -616,7 +616,7 @@ IncludeFixer::lookupCached(const SymbolID &ID) const {
   if (!Syms.empty()) {
     auto &Matched = *Syms.begin();
     if (!Matched.IncludeHeaders.empty() && Matched.Definition &&
-        Matched.CanonicalDeclaration.FileURI == Matched.Definition.FileURI)
+        Matched.CanonicalDeclaration.fileURI() == Matched.Definition.fileURI())
       Fixes = fixesForSymbols(Syms);
   }
   auto E = LookupCache.try_emplace(ID, std::move(Syms));

clang-tools-extra/clangd/Quality.cpp

@@ -281,7 +281,7 @@ computeScope(const NamedDecl *D) {
 }
 
 void SymbolRelevanceSignals::merge(const Symbol &IndexResult) {
-  SymbolURI = IndexResult.CanonicalDeclaration.FileURI;
+  SymbolURI = IndexResult.CanonicalDeclaration.fileURI();
   SymbolScope = IndexResult.Scope;
   IsInstanceMember |= isInstanceMember(IndexResult.SymInfo);
   if (!(IndexResult.Flags & Symbol::VisibleOutsideFile)) {

clang-tools-extra/clangd/XRefs.cpp

@@ -114,14 +114,14 @@ const NamedDecl *getDefinition(const NamedDecl *D) {
   return nullptr; // except cases above
 }
 
-void logIfOverflow(const SymbolLocation &Loc) {
+void logIfOverflow(const SymbolNameLocation &Loc) {
   if (Loc.Start.hasOverflow() || Loc.End.hasOverflow())
     log("Possible overflow in symbol location: {0}", Loc);
 }
 
 // Convert a SymbolLocation to LSP's Location.
 // TUPath is used to resolve the path of URI.
-std::optional<Location> toLSPLocation(const SymbolLocation &Loc,
+std::optional<Location> toLSPLocation(const SymbolNameLocation &Loc,
                                       llvm::StringRef TUPath) {
   if (!Loc)
     return std::nullopt;
@@ -134,8 +134,9 @@ std::optional<Location> toLSPLocation(const SymbolLocation &Loc,
   return *LSPLoc;
 }
 
-SymbolLocation toIndexLocation(const Location &Loc, std::string &URIStorage) {
-  SymbolLocation SymLoc;
+SymbolNameLocation toIndexLocation(const Location &Loc,
+                                   std::string &URIStorage) {
+  SymbolNameLocation SymLoc;
   URIStorage = Loc.uri.uri();
   SymLoc.FileURI = URIStorage.c_str();
   SymLoc.Start.setLine(Loc.range.start.line);
@@ -146,17 +147,17 @@ SymbolLocation toIndexLocation(const Location &Loc, std::string &URIStorage) {
 }
 
 // Returns the preferred location between an AST location and an index location.
-SymbolLocation getPreferredLocation(const Location &ASTLoc,
-                                    const SymbolLocation &IdxLoc,
-                                    std::string &Scratch) {
+SymbolNameLocation getPreferredLocation(const Location &ASTLoc,
+                                        const SymbolNameLocation &IdxLoc,
+                                        std::string &Scratch) {
   // Also use a mock symbol for the index location so that other fields (e.g.
   // definition) are not factored into the preference.
   Symbol ASTSym, IdxSym;
   ASTSym.ID = IdxSym.ID = SymbolID("mock_symbol_id");
-  ASTSym.CanonicalDeclaration = toIndexLocation(ASTLoc, Scratch);
-  IdxSym.CanonicalDeclaration = IdxLoc;
+  ASTSym.CanonicalDeclaration.NameLocation = toIndexLocation(ASTLoc, Scratch);
+  IdxSym.CanonicalDeclaration.NameLocation = IdxLoc;
   auto Merged = mergeSymbol(ASTSym, IdxSym);
-  return Merged.CanonicalDeclaration;
+  return Merged.CanonicalDeclaration.NameLocation;
 }
 
 std::vector<std::pair<const NamedDecl *, DeclRelationSet>>
@@ -309,16 +310,17 @@ std::vector<LocatedSymbol> findImplementors(llvm::DenseSet<SymbolID> IDs,
   Req.Subjects = std::move(IDs);
   std::vector<LocatedSymbol> Results;
   Index->relations(Req, [&](const SymbolID &Subject, const Symbol &Object) {
-    auto DeclLoc =
-        indexToLSPLocation(Object.CanonicalDeclaration, MainFilePath);
+    auto DeclLoc = indexToLSPLocation(Object.CanonicalDeclaration.NameLocation,
+                                      MainFilePath);
     if (!DeclLoc) {
       elog("Find overrides: {0}", DeclLoc.takeError());
       return;
     }
     Results.emplace_back();
     Results.back().Name = Object.Name.str();
     Results.back().PreferredDeclaration = *DeclLoc;
-    auto DefLoc = indexToLSPLocation(Object.Definition, MainFilePath);
+    auto DefLoc =
+        indexToLSPLocation(Object.Definition.NameLocation, MainFilePath);
     if (!DefLoc) {
       elog("Failed to convert location: {0}", DefLoc.takeError());
       return;
@@ -350,23 +352,26 @@ void enhanceLocatedSymbolsFromIndex(llvm::MutableArrayRef<LocatedSymbol> Result,
     if (R.Definition) { // from AST
       // Special case: if the AST yielded a definition, then it may not be
       // the right *declaration*. Prefer the one from the index.
-      if (auto Loc = toLSPLocation(Sym.CanonicalDeclaration, MainFilePath))
+      if (auto Loc = toLSPLocation(Sym.CanonicalDeclaration.NameLocation,
+                                   MainFilePath))
         R.PreferredDeclaration = *Loc;
 
       // We might still prefer the definition from the index, e.g. for
       // generated symbols.
       if (auto Loc = toLSPLocation(
-              getPreferredLocation(*R.Definition, Sym.Definition, Scratch),
+              getPreferredLocation(*R.Definition, Sym.Definition.NameLocation,
+                                   Scratch),
               MainFilePath))
         R.Definition = *Loc;
     } else {
-      R.Definition = toLSPLocation(Sym.Definition, MainFilePath);
+      R.Definition = toLSPLocation(Sym.Definition.NameLocation, MainFilePath);
 
       // Use merge logic to choose AST or index declaration.
-      if (auto Loc = toLSPLocation(
-              getPreferredLocation(R.PreferredDeclaration,
-                                   Sym.CanonicalDeclaration, Scratch),
-              MainFilePath))
+      if (auto Loc =
+              toLSPLocation(getPreferredLocation(
+                                R.PreferredDeclaration,
+                                Sym.CanonicalDeclaration.NameLocation, Scratch),
+                            MainFilePath))
         R.PreferredDeclaration = *Loc;
     }
   });
@@ -592,7 +597,7 @@ std::vector<LocatedSymbol> locateSymbolTextually(const SpelledWord &Word,
       return;
 
     auto MaybeDeclLoc =
-        indexToLSPLocation(Sym.CanonicalDeclaration, MainFilePath);
+        indexToLSPLocation(Sym.CanonicalDeclaration.NameLocation, MainFilePath);
     if (!MaybeDeclLoc) {
       log("locateSymbolNamedTextuallyAt: {0}", MaybeDeclLoc.takeError());
       return;
@@ -602,7 +607,8 @@ std::vector<LocatedSymbol> locateSymbolTextually(const SpelledWord &Word,
     Located.Name = (Sym.Name + Sym.TemplateSpecializationArgs).str();
     Located.ID = Sym.ID;
     if (Sym.Definition) {
-      auto MaybeDefLoc = indexToLSPLocation(Sym.Definition, MainFilePath);
+      auto MaybeDefLoc =
+          indexToLSPLocation(Sym.Definition.NameLocation, MainFilePath);
       if (!MaybeDefLoc) {
         log("locateSymbolNamedTextuallyAt: {0}", MaybeDefLoc.takeError());
         return;
@@ -1481,9 +1487,10 @@ ReferencesResult findReferences(ParsedAST &AST, Position Pos, uint32_t Limit,
           Results.HasMore = true;
           return;
         }
-        const auto LSPLocDecl =
-            toLSPLocation(Object.CanonicalDeclaration, MainFilePath);
-        const auto LSPLocDef = toLSPLocation(Object.Definition, MainFilePath);
+        const auto LSPLocDecl = toLSPLocation(
+            Object.CanonicalDeclaration.NameLocation, MainFilePath);
+        const auto LSPLocDef =
+            toLSPLocation(Object.Definition.NameLocation, MainFilePath);
         if (LSPLocDecl && LSPLocDecl != LSPLocDef) {
           ReferencesResult::Reference Result;
           Result.Loc = {std::move(*LSPLocDecl), std::nullopt};
@@ -1742,11 +1749,9 @@ static std::optional<HierarchyItem> symbolToHierarchyItem(const Symbol &S,
   HI.name = std::string(S.Name);
   HI.detail = (S.Scope + S.Name).str();
   HI.kind = indexSymbolKindToSymbolKind(S.SymInfo.Kind);
-  HI.selectionRange = Loc->range;
-  // FIXME: Populate 'range' correctly
-  // (https://github.com/clangd/clangd/issues/59).
-  HI.range = HI.selectionRange;
-  HI.uri = Loc->uri;
+  HI.selectionRange = Loc->first.range;
+  HI.range = Loc->second;
+  HI.uri = Loc->first.uri;
 
   return HI;
 }

clang-tools-extra/clangd/index/FileIndex.cpp

@@ -132,13 +132,13 @@ FileShardedIndex::FileShardedIndex(IndexFileIn Input)
   // Attribute each Symbol to both their declaration and definition locations.
   if (Index.Symbols) {
     for (const auto &S : *Index.Symbols) {
-      auto It = Shards.try_emplace(S.CanonicalDeclaration.FileURI);
+      auto It = Shards.try_emplace(S.CanonicalDeclaration.fileURI());
       It.first->getValue().Symbols.insert(&S);
       SymbolIDToFile[S.ID] = &It.first->getValue();
       // Only bother if definition file is different than declaration file.
       if (S.Definition &&
-          S.Definition.FileURI != S.CanonicalDeclaration.FileURI) {
-        auto It = Shards.try_emplace(S.Definition.FileURI);
+          S.Definition.fileURI() != S.CanonicalDeclaration.fileURI()) {
+        auto It = Shards.try_emplace(S.Definition.fileURI());
         It.first->getValue().Symbols.insert(&S);
       }
     }

clang-tools-extra/clangd/index/Index.h

@@ -99,7 +99,7 @@ struct RelationsRequest {
 
 struct ContainedRefsResult {
   /// The source location where the symbol is named.
-  SymbolLocation Location;
+  SymbolNameLocation Location;
   RefKind Kind = RefKind::Unknown;
   /// The ID of the symbol which is referred to
   SymbolID Symbol;

clang-tools-extra/clangd/index/Merge.cpp

@@ -26,7 +26,7 @@ bool isIndexAuthoritative(const SymbolIndex::IndexedFiles &Index,
   // We expect the definition to see the canonical declaration, so it seems to
   // be enough to check only the definition if it exists.
   const char *OwningFile =
-      S.Definition ? S.Definition.FileURI : S.CanonicalDeclaration.FileURI;
+      S.Definition ? S.Definition.fileURI() : S.CanonicalDeclaration.fileURI();
   return (Index(OwningFile) & IndexContents::Symbols) != IndexContents::None;
 }
 } // namespace
@@ -223,15 +223,16 @@ void MergedIndex::relations(
 
 // Returns true if \p L is (strictly) preferred to \p R (e.g. by file paths). If
 // neither is preferred, this returns false.
-static bool prefer(const SymbolLocation &L, const SymbolLocation &R) {
+static bool prefer(const SymbolDeclDefLocation &L,
+                   const SymbolDeclDefLocation &R) {
   if (!L)
     return false;
   if (!R)
     return true;
-  auto HasCodeGenSuffix = [](const SymbolLocation &Loc) {
+  auto HasCodeGenSuffix = [](const SymbolDeclDefLocation &Loc) {
     constexpr static const char *CodegenSuffixes[] = {".proto"};
     return llvm::any_of(CodegenSuffixes, [&](llvm::StringRef Suffix) {
-      return llvm::StringRef(Loc.FileURI).ends_with(Suffix);
+      return llvm::StringRef(Loc.fileURI()).ends_with(Suffix);
     });
   };
   return HasCodeGenSuffix(L) && !HasCodeGenSuffix(R);
@@ -245,9 +246,9 @@ Symbol mergeSymbol(const Symbol &L, const Symbol &R) {
   bool PreferR = R.Definition && !L.Definition;
   // Merge include headers only if both have definitions or both have no
   // definition; otherwise, only accumulate references of common includes.
-  assert(L.Definition.FileURI && R.Definition.FileURI);
+  assert(L.Definition.fileURI() && R.Definition.fileURI());
   bool MergeIncludes =
-      bool(*L.Definition.FileURI) == bool(*R.Definition.FileURI);
+      bool(*L.Definition.fileURI()) == bool(*R.Definition.fileURI());
   Symbol S = PreferR ? R : L;        // The target symbol we're merging into.
   const Symbol &O = PreferR ? L : R; // The "other" less-preferred symbol.
 

clang-tools-extra/clangd/index/Ref.h

@@ -18,6 +18,7 @@
 #include <cstdint>
 #include <set>
 #include <utility>
+#include <variant>
 
 namespace clang {
 namespace clangd {
@@ -87,7 +88,7 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &, RefKind);
 /// WARNING: Location does not own the underlying data - Copies are shallow.
 struct Ref {
   /// The source location where the symbol is named.
-  SymbolLocation Location;
+  SymbolNameLocation Location;
   RefKind Kind = RefKind::Unknown;
   /// The ID of the symbol whose definition contains this reference.
   /// For example, for a reference inside a function body, this would
@@ -185,12 +186,8 @@ template <> struct DenseMapInfo<clang::clangd::RefSlab::Builder::Entry> {
         Val.Reference.Location.Start.rep(), Val.Reference.Location.End.rep());
   }
   static bool isEqual(const Entry &LHS, const Entry &RHS) {
-    return std::tie(LHS.Symbol, LHS.Reference.Location.FileURI,
-                    LHS.Reference.Kind) ==
-               std::tie(RHS.Symbol, RHS.Reference.Location.FileURI,
-                        RHS.Reference.Kind) &&
-           LHS.Reference.Location.Start == RHS.Reference.Location.Start &&
-           LHS.Reference.Location.End == RHS.Reference.Location.End;
+    return std::tie(LHS.Symbol, LHS.Reference) ==
+           std::tie(RHS.Symbol, RHS.Reference);
   }
 };
 } // namespace llvm