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

Fangrui Song via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 12 14:57:46 PDT 2018


MaskRay added a comment.

Just my 2 cents.  Calculation of line/character for each occurrence may not take a lot of computation. cquery/ccls computes the



================
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:
> hokein wrote:
> > 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.
> After offline discussion: we don't actually plan to do math on these ever, we just send them to LSP clients.
> So removing sounds fine. We can add later if there are clear use cases.
Yes. `StartOffset` and `EndOffset` should be removed some day. a line->offset mapping should be sufficient for documents that have stale index.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45513





More information about the cfe-commits mailing list