[clangd] Get rid of ASTWorker::getCurrentFileInputs

[kadircet]

Summary

FileInputs are only written by ASTWorker thread, therefore it is safe
to read them without the lock inside that thread. It can still be read by other
threads through ASTWorker::getCurrentCompileCommand though.

This patch also gets rid of the smart pointer wrapping FileInputs as there is
never mutliple owners.

[kadircet]

• Rebase

[sammccall]

clang-tools-extra/clangd/TUScheduler.cpp

457–459

this GUARDED_BY comment is a bit misleading.
I'd be more explicit here as it's a bit unusual:
Writes and reads from unknown threads are locked.
Reads from the worker thread are not locked as it's the only writer thread.

[kadircet]

• Address comments

clang-tools-extra/clangd/TUScheduler.cpp

private:
 
   /// Determines the next action to perform.
   /// All actions that should never run are discarded.
   /// Returns a deadline for the next action. If it's expired, run now.
   /// scheduleLocked() is called again at the deadline, or if requests arrive.
   Deadline scheduleLocked();
   /// Should the first task in the queue be skipped instead of run?
   bool shouldSkipHeadLocked() const;
-  /// This is private because `FileInputs.FS` is not thread-safe and thus not
-  /// safe to share. Callers should make sure not to expose `FS` via a public
-  /// interface.
-  std::shared_ptr<const ParseInputs> getCurrentFileInputs() const;
 
   struct Request {
     llvm::unique_function<void()> Action;
     std::string Name;
     steady_clock::time_point AddTime;
     Context Ctx;
     llvm::Optional<WantDiagnostics> UpdateType;
     TUScheduler::ASTActionInvalidation InvalidationPolicy;
private:
   Notification BuiltFirstPreamble;
 
   Semaphore &Barrier;
   /// Whether the 'onMainAST' callback ran for the current FileInputs.
   bool RanASTCallback = false;
   /// Guards members used by both TUScheduler and the worker thread.
   mutable std::mutex Mutex;
   /// File inputs, currently being used by the worker.
-  /// Inputs are written and read by the worker thread, compile command can also
-  /// be consumed by clients of ASTWorker.
-  std::shared_ptr<const ParseInputs> FileInputs; /* GUARDED_BY(Mutex) */
+  /// Writes and reads from unknown threads are locked. Reads from the worker
+  /// thread are not locked, as it's the only writer.
+  ParseInputs FileInputs; /* GUARDED_BY(Mutex) */
   /// Times of recent AST rebuilds, used for UpdateDebounce computation.
   llvm::SmallVector<DebouncePolicy::clock::duration, 8>
       RebuildTimes; /* GUARDED_BY(Mutex) */
   /// Set to true to signal run() to finish processing.
   bool Done;                              /* GUARDED_BY(Mutex) */
   std::deque<Request> Requests;           /* GUARDED_BY(Mutex) */
   std::queue<Request> PreambleRequests;   /* GUARDED_BY(Mutex) */
   llvm::Optional<Request> CurrentRequest; /* GUARDED_BY(Mutex) */
ASTWorker::ASTWorker(PathRef FileName, const GlobalCompilationDatabase &CDB,
                      bool StorePreamblesInMemory, ParsingCallbacks &Callbacks)
     : IdleASTs(LRUCache), RunSync(RunSync), UpdateDebounce(UpdateDebounce),
       FileName(FileName), CDB(CDB), Callbacks(Callbacks), Barrier(Barrier),
       Done(false), Status(FileName, Callbacks),
       PreamblePeer(FileName, Callbacks, StorePreamblesInMemory,
                    // FIXME: Run PreamblePeer asynchronously once ast patching
                    // is available.
                    /*RunSync=*/true, Status, *this) {
-  auto Inputs = std::make_shared<ParseInputs>();
   // Set a fallback command because compile command can be accessed before
   // `Inputs` is initialized. Other fields are only used after initialization
   // from client inputs.
-  Inputs->CompileCommand = CDB.getFallbackCommand(FileName);
-  FileInputs = std::move(Inputs);
+  FileInputs.CompileCommand = CDB.getFallbackCommand(FileName);
 }
 
 ASTWorker::~ASTWorker() {
   // Make sure we remove the cached AST, if any.
   IdleASTs.take(this);
 #ifndef NDEBUG
   std::lock_guard<std::mutex> Lock(Mutex);
   assert(Done && "handle was not destroyed");
auto Task = [=]() mutable {
     // current implementation.
     if (auto Cmd = CDB.getCompileCommand(FileName))
       Inputs.CompileCommand = *Cmd;
     else
       // FIXME: consider using old command if it's not a fallback one.
       Inputs.CompileCommand = CDB.getFallbackCommand(FileName);
 
     bool InputsAreTheSame =
-        std::tie(FileInputs->CompileCommand, FileInputs->Contents) ==
+        std::tie(FileInputs.CompileCommand, FileInputs.Contents) ==
         std::tie(Inputs.CompileCommand, Inputs.Contents);
     // Cached AST is invalidated.
     if (!InputsAreTheSame) {
       IdleASTs.take(this);
       RanASTCallback = false;
     }
 
     // Update current inputs so that subsequent reads can see them.
     {
       std::lock_guard<std::mutex> Lock(Mutex);
-      FileInputs = std::make_shared<ParseInputs>(Inputs);
+      FileInputs = Inputs;
     }
 
     log("ASTWorker building file {0} version {1} with command {2}\n[{3}]\n{4}",
         FileName, Inputs.Version, Inputs.CompileCommand.Heuristic,
         Inputs.CompileCommand.Directory,
         llvm::join(Inputs.CompileCommand.CommandLine, " "));
 
     StoreDiags CompilerInvocationDiagConsumer;

 void ASTWorker::runWithAST(
     llvm::StringRef Name,
     llvm::unique_function<void(llvm::Expected<InputsAndAST>)> Action,
     TUScheduler::ASTActionInvalidation Invalidation) {
   auto Task = [=, Action = std::move(Action)]() mutable {
     if (isCancelled())
       return Action(llvm::make_error<CancelledError>());
     llvm::Optional<std::unique_ptr<ParsedAST>> AST = IdleASTs.take(this);
-    auto CurrentInputs = getCurrentFileInputs();
     if (!AST) {
       StoreDiags CompilerInvocationDiagConsumer;
-      std::unique_ptr<CompilerInvocation> Invocation = buildCompilerInvocation(
-          *CurrentInputs, CompilerInvocationDiagConsumer);
+      std::unique_ptr<CompilerInvocation> Invocation =
+          buildCompilerInvocation(FileInputs, CompilerInvocationDiagConsumer);
       // Try rebuilding the AST.
       vlog("ASTWorker rebuilding evicted AST to run {0}: {1} version {2}", Name,
-           FileName, CurrentInputs->Version);
+           FileName, FileInputs.Version);
       // FIXME: We might need to build a patched ast once preamble thread starts
       // running async. Currently getPossiblyStalePreamble below will always
       // return a compatible preamble as ASTWorker::update blocks.
       llvm::Optional<ParsedAST> NewAST =
           Invocation ? buildAST(FileName, std::move(Invocation),
                                 CompilerInvocationDiagConsumer.take(),
-                                *CurrentInputs, getPossiblyStalePreamble())
+                                FileInputs, getPossiblyStalePreamble())
                      : None;
       AST = NewAST ? std::make_unique<ParsedAST>(std::move(*NewAST)) : nullptr;
     }
     // Make sure we put the AST back into the LRU cache.
     auto _ = llvm::make_scope_exit(
         [&AST, this]() { IdleASTs.put(this, std::move(*AST)); });
     // Run the user-provided action.
     if (!*AST)
       return Action(llvm::make_error<llvm::StringError>(
           "invalid AST", llvm::errc::invalid_argument));
     vlog("ASTWorker running {0} on version {2} of {1}", Name, FileName,
-         CurrentInputs->Version);
-    Action(InputsAndAST{*CurrentInputs, **AST});
+         FileInputs.Version);
+    Action(InputsAndAST{FileInputs, **AST});
   };
   startTask(Name, std::move(Task), /*UpdateType=*/None, Invalidation);
 }
 
 void PreambleThread::build(Request Req) {
   assert(Req.CI && "Got preamble request with null compiler invocation");
   const ParseInputs &Inputs = Req.Inputs;
 
void ASTWorker::generateDiagnostics(
   // No need to rebuild the AST if we won't send the diagnostics.
   {
     std::lock_guard<std::mutex> Lock(PublishMu);
     if (!CanPublishResults)
       return;
   }
   // Used to check whether we can update AST cache.
   bool InputsAreLatest =
-      std::tie(FileInputs->CompileCommand, FileInputs->Contents) ==
+      std::tie(FileInputs.CompileCommand, FileInputs.Contents) ==
       std::tie(Inputs.CompileCommand, Inputs.Contents);
   // Take a shortcut and don't report the diagnostics, since they should be the
   // same. All the clients should handle the lack of OnUpdated() call anyway to
   // handle empty result from buildAST.
   // FIXME: the AST could actually change if non-preamble includes changed,
   // but we choose to ignore it.
   if (InputsAreLatest && RanASTCallback)
     return;
Requests.insert(LastUpdate.base(),
                           /*InvalidationPolicy=*/TUScheduler::NoInvalidation,
                           /*Invalidate=*/nullptr});
   Lock.unlock();
   RequestsCV.notify_all();
 }
 
 void ASTWorker::waitForFirstPreamble() const { BuiltFirstPreamble.wait(); }
 
-std::shared_ptr<const ParseInputs> ASTWorker::getCurrentFileInputs() const {
-  std::unique_lock<std::mutex> Lock(Mutex);
-  return FileInputs;
-}
-
 tooling::CompileCommand ASTWorker::getCurrentCompileCommand() const {
   std::unique_lock<std::mutex> Lock(Mutex);
-  return FileInputs->CompileCommand;
+  return FileInputs.CompileCommand;
 }
 
 std::size_t ASTWorker::getUsedBytes() const {
   // Note that we don't report the size of ASTs currently used for processing
   // the in-flight requests. We used this information for debugging purposes
   // only, so this should be fine.
   std::size_t Result = IdleASTs.getUsedBytes(this);
   if (auto Preamble = getPossiblyStalePreamble())