[PATCH] D41281: [clangd] Index-based code completion.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

A few drive-by comments, will look deeper if I have time but don't wait for me.



================
Comment at: clangd/Position.cpp:34
+  int Lines = JustBefore.count('\n');
+  int Cols = JustBefore.size() - JustBefore.rfind('\n') - 1;
+  return {Lines, Cols};
----------------
This is deep magic (how it handles npos) and I'd like to replace it with something more explicit


================
Comment at: clangd/Position.cpp:15
+
+size_t positionToOffset(llvm::StringRef Code, Position P) {
+  size_t Offset = 0;
----------------
This is kind of note-to-self, but there are things to fix here
 - this seems to handle \r\n fine, no FIXME needed
 - UTF8 comment should be `// FIXME: we're counting UTF8 bytes, LSP wants UTF16 code units`
 - we should return Code.size() when we don't find a \n
 - we should cap the normal return value at Code.size()
 - `Offset` is weird - it should consistently point after the \n (init to 0, Offset = F + 1), so -1 at the end seems like a bug


================
Comment at: clangd/Position.h:1
+//===--- Position.h - Positions in code. -------------------------*- C++-*-===//
+//
----------------
This seems like a really specific name - one possible generalization is "utilities that examine source code directly" - `SourceCode.h`?
This is just a hunch that we might have some more such logic around preambles, language detection, etc.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41281