[clang/CAS] Simplify clang::CASOptions

Replace `getOrCreateObjectStore` and `getOrCreateActionCache` with one function, `getOrCreateDatabases`.
Also re-enable the `CASOptionsTest` unit tests, by using a new function `isOnDiskCASEnabled` to check for on-disk
feature, instead of using the macro (which was not defined for that target).

Author: akyrtzi

clang/include/clang/Basic/DiagnosticCASKinds.td

@@ -9,7 +9,7 @@
 let Component = "CAS" in {
 
 def err_builtin_cas_cannot_be_initialized : Error<
-  "CAS cannot be initialized from '%0' on disk (check -fcas-path)">,
+  "CAS cannot be initialized from '%0' on disk (check -fcas-path): %1">,
   DefaultFatal;
 def err_builtin_actioncache_cannot_be_initialized : Error<
   "ActionCache cannot be initialized from '%0' on disk (check -fcas-path)">,

clang/include/clang/CAS/CASOptions.h

@@ -14,12 +14,10 @@
 #ifndef LLVM_CLANG_CAS_CASOPTIONS_H
 #define LLVM_CLANG_CAS_CASOPTIONS_H
 
-#include <memory>
+#include "llvm/ADT/SmallVector.h"
 #include <string>
-#include <vector>
 
 namespace llvm {
-class Error;
 namespace cas {
 class ActionCache;
 class ObjectStore;
@@ -30,6 +28,9 @@ namespace clang {
 
 class DiagnosticsEngine;
 
+/// Set \p Path to a reasonable default on-disk cache path for the current user.
+void getClangDefaultCachePath(llvm::SmallVectorImpl<char> &Path);
+
 /// Base class for options configuring which CAS to use. Separated for the
 /// fields where we don't need special move/copy logic.
 ///
@@ -74,8 +75,8 @@ class CASConfiguration {
 /// Options configuring which CAS to use. User-accessible fields should be
 /// defined in CASConfiguration to enable caching a CAS instance.
 ///
-/// CASOptions includes \a getOrCreateObjectStore() and \a
-/// getOrCreateActionCache() for creating CAS and ActionCache.
+/// CASOptions includes \a getOrCreateDatabases() for creating CAS and
+/// ActionCache.
 ///
 /// FIXME: The the caching is done here, instead of as a field in \a
 /// CompilerInstance, in order to ensure that \a
@@ -84,32 +85,23 @@ class CASConfiguration {
 /// it would be better to update all callers and remove it from here.
 class CASOptions : public CASConfiguration {
 public:
-  /// Get a CAS defined by the options above. Future calls will return the same
-  /// CAS instance... unless the configuration has changed, in which case a new
-  /// one will be created.
-  ///
-  /// If \p CreateEmptyCASOnFailure, returns an empty in-memory CAS on failure.
-  /// Else, returns \c nullptr on failure.
-  std::shared_ptr<llvm::cas::ObjectStore>
-  getOrCreateObjectStore(DiagnosticsEngine &Diags,
-                         bool CreateEmptyCASOnFailure = false) const;
-
-  /// Get a ActionCache defined by the options above. Future calls will return
-  /// the same ActionCache instance... unless the configuration has changed, in
-  /// which case a new one will be created.
+  /// Get a CAS & ActionCache defined by the options above. Future calls will
+  /// return the same instances... unless the configuration has changed, in
+  /// which case new ones will be created.
   ///
-  /// If \p CreateEmptyCacheOnFailure, returns an empty in-memory ActionCache on
+  /// If \p CreateEmptyDBsOnFailure, returns empty in-memory databases on
   /// failure. Else, returns \c nullptr on failure.
-  std::shared_ptr<llvm::cas::ActionCache>
-  getOrCreateActionCache(DiagnosticsEngine &Diags,
-                         bool CreateEmptyCacheOnFailures = false) const;
+  std::pair<std::shared_ptr<llvm::cas::ObjectStore>,
+            std::shared_ptr<llvm::cas::ActionCache>>
+  getOrCreateDatabases(DiagnosticsEngine &Diags,
+                       bool CreateEmptyDBsOnFailure = false) const;
 
   /// Freeze CAS Configuration. Future calls will return the same
   /// CAS instance, even if the configuration changes again later.
   ///
   /// The configuration will be wiped out to prevent it being observable or
   /// affecting the output of something that takes \a CASOptions as an input.
-  /// This also "locks in" the return value of \a getOrCreateObjectStore():
+  /// This also "locks in" the return value of \a getOrCreateDatabases():
   /// future calls will not check if the configuration has changed.
   void freezeConfig(DiagnosticsEngine &Diags);
 

clang/include/clang/Frontend/CompilerInstance.h

@@ -186,6 +186,8 @@ class CompilerInstance : public ModuleLoader {
   /// Force an output buffer.
   std::unique_ptr<llvm::raw_pwrite_stream> OutputStream;
 
+  void createCASDatabases();
+
   CompilerInstance(const CompilerInstance &) = delete;
   void operator=(const CompilerInstance &) = delete;
 public:

clang/lib/CAS/CASOptions.cpp

@@ -11,44 +11,31 @@
 #include "clang/Basic/DiagnosticCAS.h"
 #include "llvm/CAS/ActionCache.h"
 #include "llvm/CAS/ObjectStore.h"
-#include "llvm/Support/Error.h"
+#include "llvm/Support/Path.h"
 
 using namespace clang;
 using namespace llvm::cas;
 
-static std::shared_ptr<llvm::cas::ObjectStore>
-createObjectStoreWithoutPath(const CASConfiguration &Config,
-                             DiagnosticsEngine &Diags) {
-  if (Config.CASPath.empty())
-    return llvm::cas::createInMemoryCAS();
-
-  assert(Config.CASPath == "auto");
-  // Compute the path.
-  SmallString<128> Storage;
-  llvm::cas::getDefaultOnDiskCASPath(Storage);
-  StringRef Path = Storage;
-
-  // FIXME: Pass on the actual error from the CAS.
-  if (auto MaybeCAS =
-          llvm::expectedToOptional(llvm::cas::createOnDiskCAS(Path)))
-    return std::move(*MaybeCAS);
-  Diags.Report(diag::err_builtin_cas_cannot_be_initialized) << Path;
-  return nullptr;
+void clang::getClangDefaultCachePath(SmallVectorImpl<char> &Path) {
+  // FIXME: Should this return 'Error' instead of hard-failing?
+  if (!llvm::sys::path::cache_directory(Path))
+    llvm::report_fatal_error("cannot get default cache directory");
+  llvm::sys::path::append(Path, "clang-cache");
 }
 
-std::shared_ptr<llvm::cas::ObjectStore>
-CASOptions::getOrCreateObjectStore(DiagnosticsEngine &Diags,
-                                   bool CreateEmptyCASOnFailure) const {
+std::pair<std::shared_ptr<llvm::cas::ObjectStore>,
+          std::shared_ptr<llvm::cas::ActionCache>>
+CASOptions::getOrCreateDatabases(DiagnosticsEngine &Diags,
+                                 bool CreateEmptyDBsOnFailure) const {
   if (Cache.Config.IsFrozen)
-    return Cache.CAS;
+    return {Cache.CAS, Cache.AC};
 
   initCache(Diags);
-  if (Cache.CAS)
-    return Cache.CAS;
-  if (!CreateEmptyCASOnFailure)
-    return nullptr;
-  Cache.CAS = llvm::cas::createInMemoryCAS();
-  return Cache.CAS;
+  if (!Cache.CAS && CreateEmptyDBsOnFailure)
+    Cache.CAS = llvm::cas::createInMemoryCAS();
+  if (!Cache.AC && CreateEmptyDBsOnFailure)
+    Cache.AC = llvm::cas::createInMemoryActionCache();
+  return {Cache.CAS, Cache.AC};
 }
 
 void CASOptions::freezeConfig(DiagnosticsEngine &Diags) {
@@ -74,45 +61,11 @@ void CASOptions::freezeConfig(DiagnosticsEngine &Diags) {
   }
 }
 
-static std::shared_ptr<llvm::cas::ActionCache>
-createCacheWithoutPath(const CASConfiguration &Config,
-                       DiagnosticsEngine &Diags) {
-  if (Config.CASPath.empty())
-    return llvm::cas::createInMemoryActionCache();
-
-  assert(Config.CASPath == "auto");
-  // Compute the path.
-  std::string Path = getDefaultOnDiskActionCachePath();
-
-  // FIXME: Pass on the actual error from the CAS.
-  if (auto MaybeCache =
-          llvm::expectedToOptional(llvm::cas::createOnDiskActionCache(Path)))
-    return std::move(*MaybeCache);
-  Diags.Report(diag::err_builtin_actioncache_cannot_be_initialized) << Path;
-  return nullptr;
-}
-
-std::shared_ptr<llvm::cas::ActionCache>
-CASOptions::getOrCreateActionCache(DiagnosticsEngine &Diags,
-                                   bool CreateEmptyOnFailure) const {
-  if (Cache.Config.IsFrozen)
-    return Cache.AC;
-
-  initCache(Diags);
-  if (Cache.AC)
-    return Cache.AC;
-  if (!CreateEmptyOnFailure)
-    return nullptr;
-
-  Cache.CAS = Cache.CAS ? Cache.CAS : llvm::cas::createInMemoryCAS();
-  return llvm::cas::createInMemoryActionCache();
-}
-
 void CASOptions::ensurePersistentCAS() {
   assert(!IsFrozen && "Expected to check for a persistent CAS before freezing");
   switch (getKind()) {
   case UnknownCAS:
-      llvm_unreachable("Cannot ensure persistent CAS if it's unknown / frozen");
+    llvm_unreachable("Cannot ensure persistent CAS if it's unknown / frozen");
   case InMemoryCAS:
     CASPath = "auto";
     break;
@@ -128,16 +81,22 @@ void CASOptions::initCache(DiagnosticsEngine &Diags) const {
 
   Cache.Config = CurrentConfig;
   StringRef CASPath = Cache.Config.CASPath;
-  if (!CASPath.empty() && CASPath != "auto") {
-    std::pair<std::unique_ptr<ObjectStore>, std::unique_ptr<ActionCache>> DBs;
-    if (llvm::Error E =
-            createOnDiskUnifiedCASDatabases(CASPath).moveInto(DBs)) {
-      Diags.Report(diag::err_builtin_cas_cannot_be_initialized) << CASPath;
-      return;
-    }
-    std::tie(Cache.CAS, Cache.AC) = std::move(DBs);
-  } else {
-    Cache.CAS = createObjectStoreWithoutPath(Cache.Config, Diags);
-    Cache.AC = createCacheWithoutPath(Cache.Config, Diags);
+  if (CASPath.empty()) {
+    Cache.CAS = llvm::cas::createInMemoryCAS();
+    Cache.AC = llvm::cas::createInMemoryActionCache();
+    return;
+  }
+
+  SmallString<256> PathBuf;
+  if (CASPath == "auto") {
+    getClangDefaultCachePath(PathBuf);
+    CASPath = PathBuf;
+  }
+  std::pair<std::unique_ptr<ObjectStore>, std::unique_ptr<ActionCache>> DBs;
+  if (llvm::Error E = createOnDiskUnifiedCASDatabases(CASPath).moveInto(DBs)) {
+    Diags.Report(diag::err_builtin_cas_cannot_be_initialized)
+        << CASPath << toString(std::move(E));
+    return;
   }
+  std::tie(Cache.CAS, Cache.AC) = std::move(DBs);
 }

clang/lib/Frontend/CompilerInstance.cpp

@@ -902,25 +902,24 @@ llvm::vfs::OutputBackend &CompilerInstance::getOrCreateOutputBackend() {
   return getOutputBackend();
 }
 
-llvm::cas::ObjectStore &CompilerInstance::getOrCreateObjectStore() {
-  if (CAS)
-    return *CAS;
-
-  // Create a new CAS instance from the CompilerInvocation. Future calls to
+void CompilerInstance::createCASDatabases() {
+  // Create a new CAS databases from the CompilerInvocation. Future calls to
   // createFileManager() will use the same CAS.
-  CAS = getInvocation().getCASOpts().getOrCreateObjectStore(
-      getDiagnostics(),
-      /*CreateEmptyCASOnFailure=*/true);
+  std::tie(CAS, ActionCache) =
+      getInvocation().getCASOpts().getOrCreateDatabases(
+          getDiagnostics(),
+          /*CreateEmptyCASOnFailure=*/true);
+}
+
+llvm::cas::ObjectStore &CompilerInstance::getOrCreateObjectStore() {
+  if (!CAS)
+    createCASDatabases();
   return *CAS;
 }
 
 llvm::cas::ActionCache &CompilerInstance::getOrCreateActionCache() {
-  if (ActionCache)
-    return *ActionCache;
-
-  ActionCache = getInvocation().getCASOpts().getOrCreateActionCache(
-      getDiagnostics(),
-      /*CreateEmptyActionCacheOnFailure=*/true);
+  if (!ActionCache)
+    createCASDatabases();
   return *ActionCache;
 }
 
@@ -2445,4 +2444,4 @@ bool CompileCacheASTReaderHelper::readCASFileSystemRootID(StringRef RootID,
     return true;
   }
   return false;
-}
+}

clang/lib/Frontend/CompilerInvocation.cpp

@@ -1347,7 +1347,7 @@ createBaseFS(const FileSystemOptions &FSOpts, const FrontendOptions &FEOpts,
   // If no CAS was provided, create one with CASOptions.
   std::shared_ptr<llvm::cas::ObjectStore> CAS = std::move(OverrideCAS);
   if (!CAS)
-    CAS = CASOpts.getOrCreateObjectStore(Diags);
+    CAS = CASOpts.getOrCreateDatabases(Diags).first;
 
   // Helper for creating a valid (but empty) CASFS if an error is encountered.
   auto makeEmptyCASFS = [&CAS]() {
@@ -2988,7 +2988,7 @@ static void determineInputFromIncludeTree(
     Diags.Report(diag::err_fe_unable_to_load_include_tree)
         << IncludeTreeID << llvm::toString(std::move(E));
   };
-  auto CAS = CASOpts.getOrCreateObjectStore(Diags);
+  auto CAS = CASOpts.getOrCreateDatabases(Diags).first;
   if (!CAS)
     return;
   auto ID = CAS->parseID(IncludeTreeID);

clang/tools/clang-cas-test/ClangCASTest.cpp

@@ -71,7 +71,8 @@ int main(int Argc, const char **Argv) {
   Opts.CASPath = CASPath.empty() ? std::string("auto") : CASPath;
 
   auto CAS =
-      Opts.getOrCreateObjectStore(*Diags, /*CreateEmptyCASOnFailure=*/false);
+      Opts.getOrCreateDatabases(*Diags, /*CreateEmptyCASOnFailure=*/false)
+          .first;
   if (!CAS)
     return 1;
 

clang/tools/clang-scan-deps/ClangScanDeps.cpp

@@ -224,8 +224,7 @@ llvm::cl::opt<std::string>
                   llvm::cl::cat(DependencyScannerCategory));
 
 llvm::cl::opt<bool> InMemoryCAS(
-    "in-memory-cas",
-    llvm::cl::desc("Use an in-memory CAS instead of on-disk."),
+    "in-memory-cas", llvm::cl::desc("Use an in-memory CAS instead of on-disk."),
     llvm::cl::init(false), llvm::cl::cat(DependencyScannerCategory));
 
 llvm::cl::opt<std::string>
@@ -881,8 +880,7 @@ int main(int argc, const char **argv) {
         CASOpts.ensurePersistentCAS();
     }
 
-    CAS = CASOpts.getOrCreateObjectStore(Diags);
-    Cache = CASOpts.getOrCreateActionCache(Diags);
+    std::tie(CAS, Cache) = CASOpts.getOrCreateDatabases(Diags);
     if (!CAS)
       return 1;
     if (Format != ScanningOutputFormat::IncludeTree)

clang/tools/driver/cc1_main.cpp

@@ -486,11 +486,8 @@ Optional<int> CompileJobCache::initialize(CompilerInstance &Clang) {
   if (!CacheCompileJob)
     return None;
 
-  CAS = Invocation.getCASOpts().getOrCreateObjectStore(Diags);
-  if (!CAS)
-    return 1; // Exit with error!
-  Cache = Invocation.getCASOpts().getOrCreateActionCache(Diags);
-  if (!Cache)
+  std::tie(CAS, Cache) = Invocation.getCASOpts().getOrCreateDatabases(Diags);
+  if (!CAS || !Cache)
     return 1; // Exit with error!
 
   CompileJobCachingOptions CacheOpts;

clang/tools/driver/cc1depscan_main.cpp

@@ -612,14 +612,8 @@ int cc1depscan_main(ArrayRef<const char *> Argv, const char *Argv0,
   CompilerInvocation::ParseCASArgs(CASOpts, ParsedCC1Args, Diags);
   CASOpts.ensurePersistentCAS();
 
-  std::shared_ptr<llvm::cas::ObjectStore> CAS =
-      CASOpts.getOrCreateObjectStore(Diags);
-  if (!CAS)
-    return 1;
-
-  std::shared_ptr<llvm::cas::ActionCache> Cache =
-      CASOpts.getOrCreateActionCache(Diags);
-  if (!Cache)
+  auto [CAS, Cache] = CASOpts.getOrCreateDatabases(Diags);
+  if (!CAS || !Cache)
     return 1;
 
   if (int Ret = scanAndUpdateCC1(Argv0, CC1Args->getValues(), NewArgs, Diags,
@@ -866,13 +860,11 @@ int ScanServer::listen() {
   bool ProduceIncludeTree =
       ParsedCASArgs.hasArg(driver::options::OPT_fdepscan_include_tree);
 
-  std::shared_ptr<llvm::cas::ObjectStore> CAS =
-      CASOpts.getOrCreateObjectStore(Diags);
+  std::shared_ptr<llvm::cas::ObjectStore> CAS;
+  std::shared_ptr<llvm::cas::ActionCache> Cache;
+  std::tie(CAS, Cache) = CASOpts.getOrCreateDatabases(Diags);
   if (!CAS)
     reportError("cannot create CAS");
-
-  std::shared_ptr<llvm::cas::ActionCache> Cache =
-      CASOpts.getOrCreateActionCache(Diags);
   if (!Cache)
     reportError("cannot create ActionCache");