[PATCH] D45513: [clangd] Add line and column number to the index symbol.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: clangd/index/Index.h:39
   // using half-open range, [StartOffset, EndOffset).
+  // FIXME(hokein): remove these fields in favor of Position.
   unsigned StartOffset = 0;
----------------
sammccall wrote:
> I don't think we should remove them, we'll just have the same problem in reverse.
> Could position have have line/col/offset, and have Position Start, End?
As discussed offline, we decide to remove them as we don't have real use case of them (we could add them back when needed).
I removed all the references of them in clangd. And remove these fields in a separate patch.


================
Comment at: clangd/index/SymbolCollector.cpp:206
+  Result.EndPos.Line = Result.StartPos.Line;
+  Result.EndPos.Character = Result.StartPos.Character + TokenLength;
+
----------------
sammccall wrote:
> I don't think this works, tokens can be split across lines.
> 
> I believe you want to compute NameLoc.locWithOffset(TokenLength) and then decompose that into line/col.
> (getLocForEndOfToken, confusingly, is different)
Good catch. Done, added a test.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45513