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

2018-04-13 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE329997: [clangd] Add line and column number to the index symbol. (authored by hokein, committed by ). Changed prior to commit: https://reviews.llvm.org/D45513?vs=142195=142349#toc Repository: rCTE

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

2018-04-13 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments. Comment at: clangd/index/Index.h:32 +// Character offset on a line in a document (zero-based). +int Character = 0; + }; MaskRay wrote: > hokein wrote: > > sammccall wrote: > > > sammccall wrote: > > > > Column? > > > > > >

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

2018-04-12 Thread Fangrui Song via Phabricator via cfe-commits
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

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

2018-04-12 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clangd/index/Index.h:32 +// Character offset on a line in a document (zero-based). +int Character = 0; + }; hokein wrote: > sammccall wrote: > > sammccall wrote: > > > Column? > > > > > > LSP calls this

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

2018-04-12 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 142195. hokein marked 3 inline comments as done. hokein added a comment. Update the patch, address remaining issues. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45513 Files: clangd/index/Index.cpp clangd/index/Index.h

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

2018-04-12 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments. Comment at: clangd/index/Index.h:32 +// Character offset on a line in a document (zero-based). +int Character = 0; + }; sammccall wrote: > sammccall wrote: > > Column? > > > > LSP calls this "character" but this is

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

2018-04-12 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. LG, with - consider reverting the bitpacking stuff - comment about utf-16 - clang-format :) Comment at: clangd/index/SymbolCollector.cpp:202 +

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

2018-04-12 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clangd/index/Index.h:27 struct SymbolLocation { + struct Position { +// Line position in a document (zero-based). sammccall wrote: > There are 4 of these per symbol, if we can keep line + character to 32 bits >

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

2018-04-12 Thread 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

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

2018-04-12 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 142170. hokein marked 5 inline comments as done. hokein added a comment. Address review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45513 Files: clangd/index/Index.cpp clangd/index/Index.h clangd/index/SymbolCollector.cpp

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

2018-04-11 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clangd/index/Index.h:27 struct SymbolLocation { + struct Position { +// Line position in a document (zero-based). There are 4 of these per symbol, if we can keep line + character to 32 bits we save 16 bytes per

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

2018-04-11 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision. hokein added a reviewer: sammccall. Herald added subscribers: MaskRay, ioeric, jkorous-apple, ilya-biryukov, klimek. LSP is using Line & column as symbol position, clangd needs to transfer file offset to Line & column when sending results back to LSP client, which is