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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
