[PATCH] D49783: [clangd] Do not rebuild AST if inputs have not changed

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.


================
Comment at: clangd/TUScheduler.cpp:357
+
+    bool CanReuseAST = InputsAreTheSame && OldPreamble == NewPreamble;
     {
----------------
nit: `(OldPreamble == NewPreamble)`

Do you intend to compare the shared pointer instead of the values?


================
Comment at: clangd/TUScheduler.cpp:360
       std::lock_guard<std::mutex> Lock(Mutex);
+      OldPreamble.reset();
       if (NewPreamble)
----------------
Why reset?


================
Comment at: clangd/TUScheduler.cpp:373
+      // FIXME(ibiryukov): the AST could actually change if non-preamble
+      // includes changed, but we choose to ignore it.
+      // FIXME(ibiryukov): should we refresh the cache in IdleASTs for the
----------------
Do we choose to ignore this because it's rare? What are non-preamble #includes?


================
Comment at: clangd/TUScheduler.cpp:376
+      // current file at this point?
+      vlog("Skipping rebuild of the AST for {0}, inputs are the same.",
+           FileName);
----------------
I think this can be a `log`.


================
Comment at: test/clangd/extra-flags.test:26
 ---
-{"jsonrpc":"2.0","method":"textDocument/didChange","params":{"textDocument":{"uri":"test:///foo.c","version":2},"contentChanges":[{"text":"int main() { int i; return i; }"}]}}
+{"jsonrpc":"2.0","method":"textDocument/didChange","params":{"textDocument":{"uri":"test:///foo.c","version":2},"contentChanges":[{"text":"int main() { int i; return i+1; }"}]}}
 #      CHECK:  "method": "textDocument/publishDiagnostics",
----------------
Why this change?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D49783