Address code review comments.

Author: qiongsiwu

include/swift/Frontend/ModuleInterfaceLoader.h

@@ -639,7 +639,7 @@ struct SwiftInterfaceInfo {
 /// then prune the extra args, and finally obtain expandedName again.
 /// Pruning always happens before the first time we call getExpandedName().
 /// Therefore it is correct to calculate expandedName only once.
-class InterfaceModuleNameExpander {
+class InterfaceModuleOutputPathResolver {
 public:
   using ArgListTy = std::vector<std::string>;
   struct ResultTy {
@@ -660,19 +660,19 @@ class InterfaceModuleNameExpander {
   // and see pruneExtraArgs to see how it can be updated.
   ArgListTy extraArgs;
 
-  std::unique_ptr<ResultTy> expandedName;
+  std::unique_ptr<ResultTy> resolvedOutputPath;
 
   std::string getHash();
 
 public:
-  InterfaceModuleNameExpander(const StringRef moduleName,
-                              const StringRef interfacePath,
-                              const StringRef sdkPath,
-                              const CompilerInvocation &CI)
+  InterfaceModuleOutputPathResolver(const StringRef moduleName,
+                                    const StringRef interfacePath,
+                                    const StringRef sdkPath,
+                                    const CompilerInvocation &CI)
       : moduleName(moduleName), interfacePath(interfacePath), sdkPath(sdkPath),
         CI(CI), extraArgs(CI.getClangImporterOptions()
                               .getReducedExtraArgsForSwiftModuleDependency()) {}
-  ResultTy getExpandedName();
+  ResultTy getOutputPath();
   void pruneExtraArgs(std::function<void(ArgListTy &)> filter);
 };
 
@@ -751,7 +751,7 @@ struct InterfaceSubContextDelegateImpl : InterfaceSubContextDelegate {
   ~InterfaceSubContextDelegateImpl() = default;
 
   /// includes a hash of relevant key data.
-  InterfaceModuleNameExpander::ResultTy
+  InterfaceModuleOutputPathResolver::ResultTy
   getCachedOutputPath(StringRef moduleName, StringRef interfacePath,
                       StringRef sdkPath);
 };

lib/DependencyScan/ScanDependencies.cpp

@@ -93,7 +93,7 @@ class ExplicitModuleDependencyResolver {
       auto swiftTextualDeps = resolvingDepInfo.getAsSwiftInterfaceModule();
       auto &interfacePath = swiftTextualDeps->swiftInterfaceFile;
       auto SDKPath = instance.getASTContext().SearchPathOpts.getSDKPath();
-      nameExpander = std::make_unique<InterfaceModuleNameExpander>(
+      outputPathResolver = std::make_unique<InterfaceModuleOutputPathResolver>(
           moduleID.ModuleName, interfacePath, SDKPath,
           instance.getInvocation());
     }
@@ -178,13 +178,13 @@ class ExplicitModuleDependencyResolver {
       }
     }
 
-    pruneUnusedVFSOverlay();
-
     // It is necessary to update the command line to take
     // the pruned unused VFS overlay into account before remapping the
     // command line.
-    if (resolvingDepInfo.isSwiftInterfaceModule())
-      updateSwiftInterfaceCommandLine();
+    if (resolvingDepInfo.isSwiftInterfaceModule()) {
+      pruneUnusedVFSOverlay();
+      updateSwiftInterfaceModuleOutputPath();
+    }
 
     // Update the dependency in the cache with the modified command-line.
     if (resolvingDepInfo.isSwiftInterfaceModule() ||
@@ -236,8 +236,8 @@ class ExplicitModuleDependencyResolver {
         return err;
     }
 
-    if (nameExpander) {
-      auto expandedName = nameExpander->getExpandedName();
+    if (outputPathResolver) {
+      auto expandedName = outputPathResolver->getOutputPath();
       depInfo.updateModuleOutputPath(expandedName.outputPath.c_str());
       depInfo.updateContextHash(expandedName.hash.str());
     }
@@ -427,43 +427,45 @@ class ExplicitModuleDependencyResolver {
     // Filter the extra args that we use to calculate the hash of the module.
     // The key assumption is that these args are clang importer args, so we
     // do not need to check for -Xcc.
-    auto extraArgsFilter = [&](InterfaceModuleNameExpander::ArgListTy &args) {
-      bool skip = false;
-      InterfaceModuleNameExpander::ArgListTy newArgs;
-      for (auto it = args.begin(), end = args.end(); it != end; it++) {
-        if (skip) {
-          skip = false;
-          continue;
-        }
-
-        if ((it + 1) != end && isVFSOverlayFlag(*it)) {
-          if (!usedVFSOverlayPaths.contains(*(it + 1))) {
-            skip = true;
-            continue;
+    auto extraArgsFilter =
+        [&](InterfaceModuleOutputPathResolver::ArgListTy &args) {
+          bool skip = false;
+          InterfaceModuleOutputPathResolver::ArgListTy newArgs;
+          for (auto it = args.begin(), end = args.end(); it != end; it++) {
+            if (skip) {
+              skip = false;
+              continue;
+            }
+
+            if ((it + 1) != end && isVFSOverlayFlag(*it)) {
+              if (!usedVFSOverlayPaths.contains(*(it + 1))) {
+                skip = true;
+                continue;
+              }
+            }
+
+            newArgs.push_back(*it);
           }
-        }
-
-        newArgs.push_back(*it);
-      }
-      args = std::move(newArgs);
-      return;
-    };
+          args = std::move(newArgs);
+          return;
+        };
 
-    if (nameExpander)
-      nameExpander->pruneExtraArgs(extraArgsFilter);
+    if (outputPathResolver)
+      outputPathResolver->pruneExtraArgs(extraArgsFilter);
 
     commandline = std::move(resolvedCommandLine);
   }
 
-  void updateSwiftInterfaceCommandLine() {
+  void updateSwiftInterfaceModuleOutputPath() {
     // The command line needs update once we prune the unused VFS overlays.
     // The update consists of two steps.
     // 1. Obtain the output path, which includes the module hash that takes the
     // VFS pruning into account.
     // 2. Update `-o `'s value on the command line with the new output path.
 
-    assert(nameExpander && "Can only update if we hae a nameExpander.");
-    const auto &expandedName = nameExpander->getExpandedName();
+    assert(outputPathResolver &&
+           "Can only update if we have an outputPathResolver.");
+    const auto &expandedName = outputPathResolver->getOutputPath();
 
     StringRef outputName = expandedName.outputPath.str();
 
@@ -603,7 +605,7 @@ class ExplicitModuleDependencyResolver {
   ModuleDependenciesCache &cache;
   CompilerInstance &instance;
   const ModuleDependencyInfo &resolvingDepInfo;
-  std::unique_ptr<InterfaceModuleNameExpander> nameExpander;
+  std::unique_ptr<InterfaceModuleOutputPathResolver> outputPathResolver;
 
   std::optional<SwiftDependencyTracker> tracker;
   std::vector<std::string> rootIDs;

lib/Frontend/ModuleInterfaceLoader.cpp

@@ -1098,9 +1098,9 @@ class ModuleInterfaceLoaderImpl {
         requiresOSSAModules);
 
     // Compute the output path if we're loading or emitting a cached module.
-    auto expandedName = astDelegate.getCachedOutputPath(
+    auto resolvedOutputPath = astDelegate.getCachedOutputPath(
         moduleName, interfacePath, ctx.SearchPathOpts.getSDKPath());
-    auto &cachedOutputPath = expandedName.outputPath;
+    auto &cachedOutputPath = resolvedOutputPath.outputPath;
 
     // Try to find the right module for this interface, either alongside it,
     // in the cache, or in the prebuilt cache.
@@ -2005,13 +2005,13 @@ InterfaceSubContextDelegateImpl::InterfaceSubContextDelegateImpl(
 
 /// Calculate an output filename in \p genericSubInvocation's cache path that
 /// includes a hash of relevant key data.
-InterfaceModuleNameExpander::ResultTy
+InterfaceModuleOutputPathResolver::ResultTy
 InterfaceSubContextDelegateImpl::getCachedOutputPath(StringRef moduleName,
                                                      StringRef interfacePath,
                                                      StringRef sdkPath) {
-  InterfaceModuleNameExpander expander(moduleName, interfacePath, sdkPath,
-                                       genericSubInvocation);
-  return expander.getExpandedName();
+  InterfaceModuleOutputPathResolver resolver(moduleName, interfacePath, sdkPath,
+                                             genericSubInvocation);
+  return resolver.getOutputPath();
 }
 
 std::error_code
@@ -2090,11 +2090,12 @@ InterfaceSubContextDelegateImpl::runInSubCompilerInstance(StringRef moduleName,
   }
 
   // Calculate output path of the module.
-  auto expandedName = getCachedOutputPath(moduleName, interfacePath, sdkPath);
+  auto resolvedOutputPath =
+      getCachedOutputPath(moduleName, interfacePath, sdkPath);
 
   // If no specific output path is given, use the hashed output path.
   if (outputPath.empty()) {
-    outputPath = expandedName.outputPath;
+    outputPath = resolvedOutputPath.outputPath;
   }
 
   // Configure the outputs in front-end options. There must be an equal number of
@@ -2148,7 +2149,7 @@ InterfaceSubContextDelegateImpl::runInSubCompilerInstance(StringRef moduleName,
   }
 
   info.BuildArguments = BuildArgs;
-  info.Hash = expandedName.hash;
+  info.Hash = resolvedOutputPath.hash;
 
   // Run the action under the sub compiler instance.
   return action(info);
@@ -2743,22 +2744,22 @@ std::unique_ptr<ExplicitCASModuleLoader> ExplicitCASModuleLoader::create(
   return result;
 }
 
-InterfaceModuleNameExpander::ResultTy
-InterfaceModuleNameExpander::getExpandedName() {
-  if (!expandedName) {
-    expandedName = std::make_unique<ResultTy>();
-    auto &outputPath = expandedName->outputPath;
+InterfaceModuleOutputPathResolver::ResultTy
+InterfaceModuleOutputPathResolver::getOutputPath() {
+  if (!resolvedOutputPath) {
+    resolvedOutputPath = std::make_unique<ResultTy>();
+    auto &outputPath = resolvedOutputPath->outputPath;
     outputPath = CI.getClangModuleCachePath();
     llvm::sys::path::append(outputPath, moduleName);
     outputPath.append("-");
     auto hashStart = outputPath.size();
     outputPath.append(getHash());
-    expandedName->hash = outputPath.str().substr(hashStart);
+    resolvedOutputPath->hash = outputPath.str().substr(hashStart);
     outputPath.append(".");
     auto outExt = file_types::getExtension(file_types::TY_SwiftModuleFile);
     outputPath.append(outExt);
   }
-  return *expandedName;
+  return *resolvedOutputPath;
 }
 
 /// Construct a key for the .swiftmodule being generated. There is a
@@ -2771,7 +2772,7 @@ InterfaceModuleNameExpander::getExpandedName() {
 /// -- rather than making a new one and potentially filling up the cache
 /// with dead entries -- when other factors change, such as the contents of
 /// the .swiftinterface input or its dependencies.
-std::string InterfaceModuleNameExpander::getHash() {
+std::string InterfaceModuleOutputPathResolver::getHash() {
   // When doing dependency scanning for explicit module, use strict context hash
   // to ensure sound module hash.
   bool useStrictCacheHash = CI.getFrontendOptions().RequestedAction ==
@@ -2841,7 +2842,9 @@ std::string InterfaceModuleNameExpander::getHash() {
   return llvm::toString(llvm::APInt(64, H), 36, /*Signed=*/false);
 }
 
-void InterfaceModuleNameExpander::pruneExtraArgs(
+void InterfaceModuleOutputPathResolver::pruneExtraArgs(
     std::function<void(ArgListTy &)> filter) {
+  assert(!resolvedOutputPath &&
+         "Cannot prune again after the output path has been resolved.");
   filter(extraArgs);
 }