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

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Dec 15 06:29:19 PST 2017

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.

  rCTE Clang Tools Extra


More information about the cfe-commits mailing list