[clang][deps] Compute command lines and file deps on-demand

[jansvoboda11]

Summary

Although generating command lines and collecting file dependencies recently got faster, there are still benefits to be had from doing these lazily, on-demand.

This patch makes it so that the ModuleDepsGraph keeps the scanning instance alive. Instances of ModuleDeps can then use pointer to the ModuleDepCollector to compute the information lazily.

[jansvoboda11]

Rebase.

[benlangmuir]

I find this a bit hard to understand as far as object lifetime is concerned: you're storing the ScanInstance in TranslationUnitDeps, but that's a layer above the actual consumer interface, which means every consumer needs to understand how this lifetime is managed or it will have dangling references by default. You also seem to have ScanInstance stored twice -- once explicitly and once in the graph.

What do you think of an alternate design where ModuleDeps has shared_ptr<ModuleDepCollector> MDC and ModuleDepCollector has shared_ptr<CompilerInstance>? That way we don't need to explicitly expose the scan instance to the consumer, it comes with the ModuleDeps, which then keeps all the necessary memory alive?

[jansvoboda11]

In D158469#4611923, @benlangmuir wrote:

I find this a bit hard to understand as far as object lifetime is concerned: you're storing the ScanInstance in TranslationUnitDeps, but that's a layer above the actual consumer interface, which means every consumer needs to understand how this lifetime is managed or it will have dangling references by default. You also seem to have ScanInstance stored twice -- once explicitly and once in the graph.

Agreed, I'm not happy about the lifetime complexity here.

What do you think of an alternate design where ModuleDeps has shared_ptr<ModuleDepCollector> MDC and ModuleDepCollector has shared_ptr<CompilerInstance>? That way we don't need to explicitly expose the scan instance to the consumer, it comes with the ModuleDeps, which then keeps all the necessary memory alive?

I tried that approach, but found it way too easy to keep ModuleDeps around, which keep scanning instances alive, and use tons of memory.

[jansvoboda11]

Hmm, maybe we could avoid holding on to the whole CompilerInstance. For generating the command-line, we only need the CompilerInvocation. For collecting file dependencies, we could hold on to the FileManager and MemoryBuffer (and maybe offset to the input files block), and deserialize that on-demand, without keeping the whole CompilerInstance around.

[benlangmuir]

I tried that approach, but found it way too easy to keep ModuleDeps around, which keep scanning instances alive, and use tons of memory.

It seems like the problem (too easy to keep around MD) is the same either way, it's just instead of wasting memory it's leaving dangling pointers that could cause UAF if they're actually used. Where were you seeing MD held too long? I wonder if there's another way to fix that.

Hmm, maybe we could avoid holding on to the whole CompilerInstance.

This seems promising!

[jansvoboda11]

In D158469#4612012, @benlangmuir wrote:

I tried that approach, but found it way too easy to keep ModuleDeps around, which keep scanning instances alive, and use tons of memory.

It seems like the problem (too easy to keep around MD) is the same either way, it's just instead of wasting memory it's leaving dangling pointers that could cause UAF if they're actually used.

You're right. I have a version of this patch that stores std::weak_ptr<ModuleDepCollector> MDC in ModuleDeps and asserts when you're about to dereference dangling pointer. That might solve that part of the issue.

Where were you seeing MD held too long? I wonder if there's another way to fix that.

For example in clang-scan-deps, we accumulate all ModuleDeps into FullDeps::Modules. This means that at the end of the scan, that can be keeping up to NumTUs worth of CompilerInvocations alive.

Hmm, maybe we could avoid holding on to the whole CompilerInstance.

This seems promising!

OK, I'll try to explore this a little further. I'm a bit concerned that FileManager (and its caches) might still be too heavy to keep around, though.

---

I guess what makes this harder to get right is the fact that ModuleDeps and ModuleDepsGraph live on different levels. I think that maybe restructuring the API a little bit could simplify things. What I have in mind is replacing the DependencyConsumer function handleModuleDependency(ModuleDeps) with handleModuleDepsGraph(ModuleDepsGraph). We could then turn ModuleDepsGraph into an iterator over proxy objects representing what's currently called ModuleDeps. (Lifetime of these proxies would be clearly tied to that of ModuleDepsGraph.) I think we can assume that scanner clients are less tempted to "permanently" store the full ModuleDepsGraph (since that's wasteful due to cross-TU sharing), compared to ModuleDeps (which you probably want to store in some form). Clients would therefore be nudged into creating own storage-friendly version of ModuleDeps before disposing of ModuleDepsGraph, which would force them to call getBuildArguments() and getFileDeps() on our ModuleDeps proxy before disposing of the heavy-weight graph. WDYT about this direction?

clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h

 namespace tooling {
 namespace dependencies {
 
 /// A callback to lookup module outputs for "-fmodule-file=", "-o" etc.
 using LookupModuleOutputCallback =
     llvm::function_ref<std::string(const ModuleID &, ModuleOutputKind)>;
 
 /// Graph of modular dependencies.
-using ModuleDepsGraph = std::vector<ModuleDeps>;
+class ModuleDepsGraph {
+  /// This is keeping the scan instance alive so that \c ModuleDeps can use it
+  /// to compute some properties lazily.
+  std::shared_ptr<CompilerInstance> ScanInstance;
+
+public:
+  ModuleDepsGraph(std::shared_ptr<CompilerInstance> ScanInstance = nullptr)
+      : ScanInstance(std::move(ScanInstance)) {}
+
+  std::vector<ModuleDeps> MDs;
+};
 
 /// The full dependencies and module graph for a specific input.
 struct TranslationUnitDeps {
   /// The graph of direct and transitive modular dependencies.
   ModuleDepsGraph ModuleGraph;
 
   /// The identifier of the C++20 module this translation unit exports.
   ///
public:
   void handleDirectModuleDependency(ModuleID ID) override {
     DirectModuleDeps.push_back(ID);
   }
 
   void handleContextHash(std::string Hash) override {
     ContextHash = std::move(Hash);
   }
 
+  void handleScanInstance(std::shared_ptr<CompilerInstance> CI) override {
+    ScanInstance = std::move(CI);
+  }
+
   TranslationUnitDeps takeTranslationUnitDeps();
   ModuleDepsGraph takeModuleGraphDeps();
 
 private:
   std::vector<std::string> Dependencies;
   std::vector<PrebuiltModuleDep> PrebuiltModuleDeps;
   llvm::MapVector<ModuleID, ModuleDeps> ClangModuleDeps;
   std::vector<ModuleID> DirectModuleDeps;
   std::vector<Command> Commands;
   std::string ContextHash;
+  std::shared_ptr<CompilerInstance> ScanInstance;
   std::vector<std::string> OutputPaths;
   const llvm::DenseSet<ModuleID> &AlreadySeen;
 };
 
 /// A simple dependency action controller that uses a callback. If no callback
 /// is provided, it is assumed that looking up module outputs is unreachable.
 class CallbackActionController : public DependencyActionController {
 public:

clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h

public:
   virtual void handleFileDependency(StringRef Filename) = 0;
 
   virtual void handlePrebuiltModuleDependency(PrebuiltModuleDep PMD) = 0;
 
   virtual void handleModuleDependency(ModuleDeps MD) = 0;
 
   virtual void handleDirectModuleDependency(ModuleID MD) = 0;
 
+  virtual void handleScanInstance(std::shared_ptr<CompilerInstance> CI) = 0;
+
   virtual void handleContextHash(std::string Hash) = 0;
 };
 
 /// Dependency scanner callbacks that are used during scanning to influence the
 /// behaviour of the scan - for example, to customize the scanned invocations.
 class DependencyActionController {
 public:
   virtual ~DependencyActionController();

clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h

enum class ModuleOutputKind {
   DependencyFile,
   /// The null-separated list of names to use as the targets in the dependency
   /// file, if any. Defaults to the value of \c ModuleFile, as in the driver.
   DependencyTargets,
   /// The path of the serialized diagnostic file (.dia), if any.
   DiagnosticSerializationFile,
 };
 
-struct ModuleDeps {
+class ModuleDepCollector;
+class ModuleDepCollectorPP;
+
+class ModuleDeps {
+  /// This facilitates the lazy computation of file deps and build arguments.
+  ModuleDepCollector *MDC;
+  /// Storage for the lazily-computed file dependencies.
+  std::optional<llvm::StringSet<>> FileDeps;
+  /// Storage for the lazily-computed build arguments.
+  std::optional<std::vector<std::string>> BuildArguments;
+
+  friend ModuleDepCollector;
+  friend ModuleDepCollectorPP;
+
+public:
   /// The identifier of the module.
   ModuleID ID;
 
   /// Whether this is a "system" module.
   bool IsSystem;
 
   /// The path to the modulemap file which defines this module.
   ///
   /// This can be used to explicitly build this module. This file will
   /// additionally appear in \c FileDeps as a dependency.
   std::string ClangModuleMapFile;
 
-  /// A collection of absolute paths to files that this module directly depends
-  /// on, not including transitive dependencies.
-  llvm::StringSet<> FileDeps;
+  /// Compute/get the set of absolute paths to files that this module directly
+  /// depends on, not including transitive dependencies. Must be first called
+  /// during the lifetime of the parent dependency graph.
+  const llvm::StringSet<> &getFileDeps();
 
   /// A collection of absolute paths to module map files that this module needs
   /// to know about. The ordering is significant.
   std::vector<std::string> ModuleMapFileDeps;
 
   /// A collection of prebuilt modular dependencies this module directly depends
   /// on, not including transitive dependencies.
   std::vector<PrebuiltModuleDep> PrebuiltModuleDeps;
 
   /// A list of module identifiers this module directly depends on, not
   /// including transitive dependencies.
   ///
   /// This may include modules with a different context hash when it can be
   /// determined that the differences are benign for this compilation.
   std::vector<ModuleID> ClangModuleDeps;
 
-  /// Compiler invocation that can be used to build this module. Does not
-  /// include argv[0].
-  std::vector<std::string> BuildArguments;
+  /// Compute/get the compiler invocation that can be used to build this module.
+  /// Does not include argv[0]. Must be first called during the lifetime of the
+  /// parent dependency graph.
+  const std::vector<std::string> &getBuildArguments();
 };
 
-class ModuleDepCollector;
-
 /// Callback that records textual includes and direct modular includes/imports
 /// during preprocessing. At the end of the main file, it also collects
 /// transitive modular dependencies and passes everything to the
 /// \c DependencyConsumer of the parent \c ModuleDepCollector.
 class ModuleDepCollectorPP final : public PPCallbacks {
 public:
   ModuleDepCollectorPP(ModuleDepCollector &MDC) : MDC(MDC) {}
 
public:
   void attachToASTReader(ASTReader &R) override;
 
   /// Apply any changes implied by the discovered dependencies to the given
   /// invocation, (e.g. disable implicit modules, add explicit module paths).
   void applyDiscoveredDependencies(CompilerInvocation &CI);
 
 private:
   friend ModuleDepCollectorPP;
+  friend ModuleDeps;
+
+  /// Information we keep to be able to compute some ModuleDeps info lazily.
+  struct LazyModuleDepsInfo {
+    serialization::ModuleFile *MF;
+    CompilerInvocation CI;
+  };
 
   /// The compiler instance for scanning the current translation unit.
   CompilerInstance &ScanInstance;
   /// The consumer of collected dependency information.
   DependencyConsumer &Consumer;
   /// Callbacks for computing dependency information.
   DependencyActionController &Controller;
   /// Path to the main source file.
   std::string MainFile;
   /// Hash identifying the compilation conditions of the current TU.
   std::string ContextHash;
   /// Non-modular file dependencies. This includes the main source file and
   /// textually included header files.
   std::vector<std::string> FileDeps;
   /// Direct and transitive modular dependencies of the main source file.
   llvm::MapVector<const Module *, std::unique_ptr<ModuleDeps>> ModularDeps;
   /// Secondary mapping for \c ModularDeps allowing lookup by ModuleID without
   /// a preprocessor. Storage owned by \c ModularDeps.
   llvm::DenseMap<ModuleID, ModuleDeps *> ModuleDepsByID;
+  /// Mapping for lazy computation of some ModuleDeps info.
+  llvm::DenseMap<ModuleID, LazyModuleDepsInfo> LazyModuleDepsInfoByID;
   /// Direct modular dependencies that have already been built.
   llvm::MapVector<const Module *, PrebuiltModuleDep> DirectPrebuiltModularDeps;
   /// Working set of direct modular dependencies.
   llvm::SetVector<const Module *> DirectModularDeps;
   /// Options that control the dependency output generation.
   std::unique_ptr<DependencyOutputOptions> Opts;
   /// The original Clang invocation passed to dependency scanner.
   CompilerInvocation OriginalInvocation;
private:
 
   /// Add module map files to the invocation, if needed.
   void addModuleMapFiles(CompilerInvocation &CI,
                          ArrayRef<ModuleID> ClangModuleDeps) const;
   /// Add module files (pcm) to the invocation, if needed.
   void addModuleFiles(CompilerInvocation &CI,
                       ArrayRef<ModuleID> ClangModuleDeps) const;
 
+  /// Compute the file deps and store them into \c MD.
+  void addFileDeps(ModuleDeps &MD);
+  /// Compute the build arguments and store them into \c MD.
+  void addBuildArguments(ModuleDeps &MD);
+
   /// Add paths that require looking up outputs to the given dependencies.
   void addOutputPaths(CompilerInvocation &CI, ModuleDeps &Deps);
 
   /// Compute the context hash for \p Deps, and create the mapping
   /// \c ModuleDepsByID[Deps.ID] = &Deps.
   void associateWithContextHash(const CompilerInvocation &CI, ModuleDeps &Deps);
 };
 

clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp

public:
 
   // These are ignored for the make format as it can't support the full
   // set of deps, and handleFileDependency handles enough for implicitly
   // built modules to work.
   void handlePrebuiltModuleDependency(PrebuiltModuleDep PMD) override {}
   void handleModuleDependency(ModuleDeps MD) override {}
   void handleDirectModuleDependency(ModuleID ID) override {}
   void handleContextHash(std::string Hash) override {}
+  void handleScanInstance(std::shared_ptr<CompilerInstance> CI) override {}
 
   void printDependencies(std::string &S) {
     assert(Opts && "Handled dependency output options.");
 
     class DependencyPrinter : public DependencyFileGenerator {
     public:
       DependencyPrinter(DependencyOutputOptions &Opts,
                         ArrayRef<std::string> Dependencies)

 
 TranslationUnitDeps FullDependencyConsumer::takeTranslationUnitDeps() {
   TranslationUnitDeps TU;
 
   TU.ID.ContextHash = std::move(ContextHash);
   TU.FileDeps = std::move(Dependencies);
   TU.PrebuiltModuleDeps = std::move(PrebuiltModuleDeps);
   TU.Commands = std::move(Commands);
-
-  for (auto &&M : ClangModuleDeps) {
-    auto &MD = M.second;
-    // TODO: Avoid handleModuleDependency even being called for modules
-    //   we've already seen.
-    if (AlreadySeen.count(M.first))
-      continue;
-    TU.ModuleGraph.push_back(std::move(MD));
-  }
+  TU.ModuleGraph = takeModuleGraphDeps();
   TU.ClangModuleDeps = std::move(DirectModuleDeps);
 
   return TU;
 }
 
 ModuleDepsGraph FullDependencyConsumer::takeModuleGraphDeps() {
-  ModuleDepsGraph ModuleGraph;
+  ModuleDepsGraph ModuleGraph(std::move(ScanInstance));
 
   for (auto &&M : ClangModuleDeps) {
     auto &MD = M.second;
     // TODO: Avoid handleModuleDependency even being called for modules
     //   we've already seen.
     if (AlreadySeen.count(M.first))
       continue;
-    ModuleGraph.push_back(std::move(MD));
+    ModuleGraph.MDs.push_back(std::move(MD));
   }
 
   return ModuleGraph;
 }
 
 CallbackActionController::~CallbackActionController() {}

clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp

if (Scanned) {
       // FIXME: to support multi-arch builds, each arch requires a separate scan
       setLastCC1Arguments(std::move(OriginalInvocation));
       return true;
     }
 
     Scanned = true;
 
     // Create a compiler instance to handle the actual work.
-    ScanInstanceStorage.emplace(std::move(PCHContainerOps));
+    ScanInstanceStorage =
+        std::make_shared<CompilerInstance>(std::move(PCHContainerOps));
     CompilerInstance &ScanInstance = *ScanInstanceStorage;
     ScanInstance.setInvocation(std::move(Invocation));
 
     // Create the compiler's actual diagnostics engine.
     sanitizeDiagOpts(ScanInstance.getDiagnosticOpts());
     ScanInstance.createDiagnostics(DiagConsumer, /*ShouldOwnClient=*/false);
     if (!ScanInstance.hasDiagnostics())
       return false;
bool runInvocation(std::shared_ptr<CompilerInvocation> Invocation,
     else
       Action = std::make_unique<ReadPCHAndPreprocessAction>();
 
     const bool Result = ScanInstance.ExecuteAction(*Action);
 
     if (Result)
       setLastCC1Arguments(std::move(OriginalInvocation));
 
+    Consumer.handleScanInstance(ScanInstanceStorage);
+
     return Result;
   }
 
   bool hasScanned() const { return Scanned; }
 
   /// Take the cc1 arguments corresponding to the most recent invocation used
   /// with this action. Any modifications implied by the discovered dependencies
   /// will have already been applied.
private:
   DependencyConsumer &Consumer;
   DependencyActionController &Controller;
   llvm::IntrusiveRefCntPtr<DependencyScanningWorkerFilesystem> DepFS;
   ScanningOutputFormat Format;
   bool OptimizeArgs;
   bool EagerLoadModules;
   bool DisableFree;
   std::optional<StringRef> ModuleName;
-  std::optional<CompilerInstance> ScanInstanceStorage;
+  std::shared_ptr<CompilerInstance> ScanInstanceStorage;
   std::shared_ptr<ModuleDepCollector> MDC;
   std::vector<std::string> LastCC1Arguments;
   bool Scanned = false;
 };
 
 } // end anonymous namespace
 
 DependencyScanningWorker::DependencyScanningWorker(

clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp

if (ModuleMap) {
     SmallString<128> Path = ModuleMap->getNameAsRequested();
     ModMapInfo.canonicalizeModuleMapPath(Path);
     MD.ClangModuleMapFile = std::string(Path);
   }
 
   serialization::ModuleFile *MF =
       MDC.ScanInstance.getASTReader()->getModuleManager().lookup(
           M->getASTFile());
-  MDC.ScanInstance.getASTReader()->visitInputFileInfos(
-      *MF, /*IncludeSystem=*/true,
-      [&](const serialization::InputFileInfo &IFI, bool IsSystem) {
-        // __inferred_module.map is the result of the way in which an implicit
-        // module build handles inferred modules. It adds an overlay VFS with
-        // this file in the proper directory and relies on the rest of Clang to
-        // handle it like normal. With explicitly built modules we don't need
-        // to play VFS tricks, so replace it with the correct module map.
-        if (StringRef(IFI.Filename).endswith("__inferred_module.map")) {
-          MDC.addFileDep(MD, ModuleMap->getName());
-          return;
-        }
-        MDC.addFileDep(MD, IFI.Filename);
-      });
 
   llvm::DenseSet<const Module *> SeenDeps;
   addAllSubmodulePrebuiltDeps(M, MD, SeenDeps);
   addAllSubmoduleDeps(M, MD, SeenDeps);
   addAllAffectingClangModules(M, MD, SeenDeps);
 
   MDC.ScanInstance.getASTReader()->visitInputFileInfos(
       *MF, /*IncludeSystem=*/true,
CompilerInvocation CI = MDC.makeInvocationForModuleBuildWithoutOutputs(
                                    *MDC.ScanInstance.getASTReader(), *MF);
       });
 
   MDC.associateWithContextHash(CI, MD);
 
   // Finish the compiler invocation. Requires dependencies and the context hash.
   MDC.addOutputPaths(CI, MD);
 
-  MD.BuildArguments = CI.getCC1CommandLine();
+  // Wire up lazy info computation.
+  MD.MDC = &MDC;
+  MDC.LazyModuleDepsInfoByID.insert({MD.ID, {MF, std::move(CI)}});
 
   return MD.ID;
 }
 
 static void forEachSubmoduleSorted(const Module *M,
                                    llvm::function_ref<void(const Module *)> F) {
   // Submodule order depends on order of header includes for inferred submodules
   // we don't care about the exact order, so sort so that it's consistent across
void ModuleDepCollector::addFileDep(StringRef Path) {
   llvm::SmallString<256> Storage;
   Path = makeAbsoluteAndPreferred(ScanInstance, Path, Storage);
   FileDeps.push_back(std::string(Path));
 }
 
 void ModuleDepCollector::addFileDep(ModuleDeps &MD, StringRef Path) {
   llvm::SmallString<256> Storage;
   Path = makeAbsoluteAndPreferred(ScanInstance, Path, Storage);
-  MD.FileDeps.insert(Path);
+  MD.FileDeps->insert(Path);
+}
+
+void ModuleDepCollector::addFileDeps(ModuleDeps &MD) {
+  auto It = LazyModuleDepsInfoByID.find(MD.ID);
+  assert(It != LazyModuleDepsInfoByID.end());
+
+  MD.FileDeps = llvm::StringSet<>{};
+
+  ScanInstance.getASTReader()->visitInputFileInfos(
+      *It->second.MF, /*IncludeSystem=*/true,
+      [&](const serialization::InputFileInfo &IFI, bool IsSystem) {
+        // __inferred_module.map is the result of the way in which an implicit
+        // module build handles inferred modules. It adds an overlay VFS with
+        // this file in the proper directory and relies on the rest of Clang to
+        // handle it like normal. With explicitly built modules we don't need
+        // to play VFS tricks, so replace it with the correct module map.
+        if (StringRef(IFI.Filename).endswith("__inferred_module.map")) {
+          FileManager &FileMgr = ScanInstance.getFileManager();
+          auto ModuleMap = FileMgr.getOptionalFileRef(MD.ClangModuleMapFile);
+          assert(ModuleMap && "Module map file of a dependency still exists");
+          addFileDep(MD, ModuleMap->getName());
+          return;
+        }
+        addFileDep(MD, IFI.Filename);
+      });
+}
+
+void ModuleDepCollector::addBuildArguments(ModuleDeps &MD) {
+  auto It = LazyModuleDepsInfoByID.find(MD.ID);
+  assert(It != LazyModuleDepsInfoByID.end());
+  MD.BuildArguments = It->second.CI.getCC1CommandLine();
+}
+
+const llvm::StringSet<> &ModuleDeps::getFileDeps() {
+  if (FileDeps)
+    return *FileDeps;
+  MDC->addFileDeps(*this);
+  return *FileDeps;
+}
+
+const std::vector<std::string> &ModuleDeps::getBuildArguments() {
+  if (BuildArguments)
+    return *BuildArguments;
+  MDC->addBuildArguments(*this);
+  return *BuildArguments;
 }

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

void mergeDeps(StringRef Input, TranslationUnitDeps TUDeps,
     ID.Commands = std::move(TUDeps.Commands);
 
     assert(InputIndex < Inputs.size() && "Input index out of bounds");
     assert(Inputs[InputIndex].FileName.empty() && "Result already populated");
     Inputs[InputIndex] = std::move(ID);
   }
 
   void mergeDeps(ModuleDepsGraph Graph, size_t InputIndex) {
+    std::vector<ModuleDeps *> NewMDs;
+    {
       std::unique_lock<std::mutex> ul(Lock);
-    for (const ModuleDeps &MD : Graph) {
+      for (const ModuleDeps &MD : Graph.MDs) {
         auto I = Modules.find({MD.ID, 0});
         if (I != Modules.end()) {
           I->first.InputIndex = std::min(I->first.InputIndex, InputIndex);
           continue;
         }
-      Modules.insert(I, {{MD.ID, InputIndex}, std::move(MD)});
+        auto NewIt = Modules.insert(I, {{MD.ID, InputIndex}, std::move(MD)});
+        NewMDs.push_back(&NewIt->second);
+      }
+    }
+    // Eagerly compute the lazy members before the graph goes out of scope.
+    // This is somewhat costly, so do it outside the critical section.
+    for (ModuleDeps *MD : NewMDs) {
+      (void)MD->getBuildArguments();
+      (void)MD->getFileDeps();
     }
   }
 
   bool roundTripCommand(ArrayRef<std::string> ArgStrs,
                         DiagnosticsEngine &Diags) {
     if (ArgStrs.empty() || ArgStrs[0] != "-cc1")
       return false;
     SmallVector<const char *> Args;
     for (const std::string &Arg : ArgStrs)
       Args.push_back(Arg.c_str());
     return !CompilerInvocation::checkCC1RoundTrip(Args, Diags);
   }
 
   // Returns \c true if any command lines fail to round-trip. We expect
   // commands already be canonical when output by the scanner.
   bool roundTripCommands(raw_ostream &ErrOS) {
     IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts = new DiagnosticOptions{};
     TextDiagnosticPrinter DiagConsumer(ErrOS, &*DiagOpts);
     IntrusiveRefCntPtr<DiagnosticsEngine> Diags =
         CompilerInstance::createDiagnostics(&*DiagOpts, &DiagConsumer,
                                             /*ShouldOwnClient=*/false);
 
     for (auto &&M : Modules)
-      if (roundTripCommand(M.second.BuildArguments, *Diags))
+      if (roundTripCommand(M.second.getBuildArguments(), *Diags))
         return true;
 
     for (auto &&I : Inputs)
       for (const auto &Cmd : I.Commands)
         if (roundTripCommand(Cmd.Arguments, *Diags))
           return true;
 
     return false;
void printFullOutput(raw_ostream &OS) {
     using namespace llvm::json;
 
     Array OutModules;
     for (auto &&ModID : ModuleIDs) {
       auto &MD = Modules[ModID];
       Object O{
           {"name", MD.ID.ModuleName},
           {"context-hash", MD.ID.ContextHash},
-          {"file-deps", toJSONSorted(MD.FileDeps)},
+          {"file-deps", toJSONSorted(MD.getFileDeps())},
           {"clang-module-deps", toJSONSorted(MD.ClangModuleDeps)},
           {"clang-modulemap-file", MD.ClangModuleMapFile},
-          {"command-line", MD.BuildArguments},
+          {"command-line", MD.getBuildArguments()},
       };
       OutModules.push_back(std::move(O));
     }
 
     Array TUs;
     for (auto &&I : Inputs) {
       Array Commands;
       if (I.DriverCommandLine.empty()) {