[PATCH] D44272: [clangd] Support incremental document syncing

Simon Marchi via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Mar 9 08:43:37 PST 2018


simark marked 2 inline comments as done.
simark added inline comments.


================
Comment at: clangd/ClangdServer.h:159
   /// constructor will receive onDiagnosticsReady callback.
   void addDocument(PathRef File, StringRef Contents,
+                   WantDiagnostics WantDiags = WantDiagnostics::Auto,
----------------
simark wrote:
> ilya-biryukov wrote:
> > I don't think we should do this on `ClangdServer` level. We will have to copy the whole file anyway before passing it to clang, so there are no performance wins here and it complicates the interface.
> > I suggest we update the document text from diffs on `ClangdLSPServer` level and continue passing the whole document to `ClangdServer`.
> > It would mean `DraftMgr` will need to move to `ClangdLSPServer`, but it's fine.
> > 
> `ClangdServer` also needs `DraftMgr` for `forceReparse` and `reparseOpenedFiles`.  I guess that too would move to `ClangdLSPServer`?  I'm just not sure how to adapt the unit tests, since we don't have unittests using `ClangdLSPServer` (yet).
Actually, `forceReparse` doesn't really need `DraftMgr` (it's only used for the assert), only `reparseOpenedFiles` needs it.  So in the test, we can manually call `forceReparse` on all the files by hand... this just means that `reparseOpenedFiles` won't be tested anymore.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44272





More information about the cfe-commits mailing list