[PATCH] D64475: [clangd] Duplicate lines of semantic highlightings sent removed.

Haojian Wu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 30 04:52:16 PDT 2019


hokein added inline comments.


================
Comment at: clang-tools-extra/clangd/ClangdServer.cpp:81
+    unsigned FileSize = SM.getFileEntryForID(MainFileID)->getSize();
+    int NLines = AST.getSourceManager().getLineNumber(MainFileID, FileSize);
+
----------------
from the comment of the getLineNumber, this is not cheap to call this method. We may use `SM.getBufferData(MainFileID).count('\n')` to count the line numbers.

```
// This requires building and caching a table of line offsets for the
/// MemoryBuffer, so this is not cheap: use only when about to emit a
/// diagnostic.
```


================
Comment at: clang-tools-extra/clangd/ClangdServer.h:55
   /// Called by ClangdServer when some \p Highlightings for \p File are ready.
-  virtual void
-  onHighlightingsReady(PathRef File,
-                       std::vector<HighlightingToken> Highlightings) {}
+  /// \p NLines are the number of lines in the file, needed to make sure that
+  /// any diffing does not add lines beyond EOF.
----------------
nit: drop the `needed ...`, this is an interface, don't mention the details of subclass (diffs are subclass implementation details).

Maybe name it "NumLines"? 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64475/new/

https://reviews.llvm.org/D64475





More information about the cfe-commits mailing list