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 symbol. That looks like it might still be ~10% of non-string size. WDYT? (12 bits for column and 20 bits for line seems like a reasonable guess) ================ Comment at: clangd/index/Index.h:28 + struct Position { + // Line position in a document (zero-based). + int Line = 0; ---------------- These comments seem pretty obvious, I think `int Line=0, Character=0 // zero-based` is enough, but up to you. I would like a comment explaining why we're storing these redundant representations though. e.g. `// Storing Line/Character allows us to build LSP responses without reading the file content.` ================ Comment at: clangd/index/Index.h:32 + // Character offset on a line in a document (zero-based). + int Character = 0; + }; ---------------- Column? LSP calls this "character" but this is nonstandard and I find it very confusing with offset. ================ 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; ---------------- 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? ================ Comment at: clangd/index/SymbolCollector.cpp:206 + Result.EndPos.Line = Result.StartPos.Line; + Result.EndPos.Character = Result.StartPos.Character + TokenLength; + ---------------- 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) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45513 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits