[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.

  rCTE Clang Tools Extra


More information about the cfe-commits mailing list