Merge pull request #76732 from cachemeifyoucan/eng/PR-136682810

[Macro] Precise macro plugin dependency during scanning

Author: cachemeifyoucan

include/swift/AST/DiagnosticsFrontend.def

@@ -127,6 +127,9 @@ ERROR(error_no_source_location_scope_map,none,
 ERROR(error_load_plugin_executable,none,
       "invalid value '%0' in '-load-plugin-executable'; "
       "make sure to use format '<plugin path>#<module names>'", (StringRef))
+ERROR(error_load_resolved_plugin,none,
+      "invalid value '%0' in '-load-resolved-plugin'; "
+      "make sure to use format '<library path>#<plugin path>#<module names>' where library and plugin path can't both be empty", (StringRef))
 
 NOTE(note_valid_swift_versions, none,
      "valid arguments to '-swift-version' are %0", (StringRef))

include/swift/AST/SearchPathOptions.h

@@ -214,17 +214,24 @@ class PluginSearchOption {
     std::string SearchPath;
     std::string ServerPath;
   };
+  struct ResolvedPluginConfig {
+    std::string LibraryPath;
+    std::string ExecutablePath;
+    std::vector<std::string> ModuleNames;
+  };
 
   enum class Kind : uint8_t {
     LoadPluginLibrary,
     LoadPluginExecutable,
     PluginPath,
     ExternalPluginPath,
+    ResolvedPluginConfig,
   };
 
 private:
-  using Members = ExternalUnionMembers<LoadPluginLibrary, LoadPluginExecutable,
-                                       PluginPath, ExternalPluginPath>;
+  using Members =
+      ExternalUnionMembers<LoadPluginLibrary, LoadPluginExecutable, PluginPath,
+                           ExternalPluginPath, ResolvedPluginConfig>;
   static Members::Index getIndexForKind(Kind kind) {
     switch (kind) {
     case Kind::LoadPluginLibrary:
@@ -235,6 +242,8 @@ class PluginSearchOption {
       return Members::indexOf<PluginPath>();
     case Kind::ExternalPluginPath:
       return Members::indexOf<ExternalPluginPath>();
+    case Kind::ResolvedPluginConfig:
+      return Members::indexOf<ResolvedPluginConfig>();
     }
   };
   using Storage = ExternalUnion<Kind, Members, getIndexForKind>;
@@ -258,6 +267,10 @@ class PluginSearchOption {
       : kind(Kind::ExternalPluginPath) {
     storage.emplace<ExternalPluginPath>(kind, v);
   }
+  PluginSearchOption(const ResolvedPluginConfig &v)
+      : kind(Kind::ResolvedPluginConfig) {
+    storage.emplace<ResolvedPluginConfig>(kind, v);
+  }
   PluginSearchOption(const PluginSearchOption &o) : kind(o.kind) {
     storage.copyConstruct(o.kind, o.storage);
   }

include/swift/Option/Options.td

@@ -2106,6 +2106,14 @@ def load_plugin_executable:
            "of module names where the macro types are declared">,
   MetaVarName<"<path>#<module-names>">;
 
+def load_resolved_plugin:
+  Separate<["-"], "load-resolved-plugin">, Group<plugin_search_Group>,
+  Flags<[FrontendOption, DoesNotAffectIncrementalBuild, ArgumentIsPath]>,
+  HelpText<"Path to resolved plugin configuration and a comma-separated list "
+           "of module names where the macro types are declared. Library path "
+           "and exectuable path can be empty if not used">,
+  MetaVarName<"<library-path>#<executable-path>#<module-names>">;
+
 def in_process_plugin_server_path : Separate<["-"], "in-process-plugin-server-path">,
   Flags<[FrontendOption, ArgumentIsPath]>,
   HelpText<"Path to dynamic library plugin server">;

lib/AST/PluginLoader.cpp

@@ -83,18 +83,17 @@ PluginLoader::getPluginMap() {
   // Helper function to try inserting an entry if there's no existing entry
   // associated with the module name.
   auto try_emplace = [&](StringRef moduleName, StringRef libPath,
-                         StringRef execPath) {
+                         StringRef execPath, bool overwrite = false) {
     auto moduleNameIdentifier = Ctx.getIdentifier(moduleName);
-    if (map.find(moduleNameIdentifier) != map.end()) {
-      // Specified module name is already in the map.
+    if (map.find(moduleNameIdentifier) != map.end() && !overwrite) {
+      // Specified module name is already in the map and no need to overwrite
+      // the current value.
       return;
     }
 
     libPath = libPath.empty() ? "" : Ctx.AllocateCopy(libPath);
     execPath = execPath.empty() ? "" : Ctx.AllocateCopy(execPath);
-    auto result = map.insert({moduleNameIdentifier, {libPath, execPath}});
-    assert(result.second);
-    (void)result;
+    map[moduleNameIdentifier] = {libPath, execPath};
   };
 
   auto fs = getPluginLoadingFS(Ctx);
@@ -150,6 +149,18 @@ PluginLoader::getPluginMap() {
       }
       continue;
     }
+
+    // '-load-resolved-plugin <library path>#<server path>#<module name>,...'.
+    case PluginSearchOption::Kind::ResolvedPluginConfig: {
+      auto &val = entry.get<PluginSearchOption::ResolvedPluginConfig>();
+      // Respect resolved plugin config above other search path, and it can
+      // overwrite plugins found by other options or previous resolved
+      // configuration.
+      for (auto &moduleName : val.ModuleNames)
+        try_emplace(moduleName, val.LibraryPath, val.ExecutablePath,
+                    /*overwrite*/ true);
+      continue;
+    }
     }
     llvm_unreachable("unhandled PluginSearchOption::Kind");
   }

lib/DependencyScan/ScanDependencies.cpp

@@ -160,25 +160,6 @@ parseBatchScanInputFile(ASTContext &ctx, StringRef batchInputPath,
   return result;
 }
 
-static void removeMacroSearchPaths(std::vector<std::string> &cmd) {
-  // Macro search path options.
-  static const llvm::StringSet<> macroSearchOptions = {
-      "-plugin-path",
-      "-external-plugin-path",
-      "-load-plugin-library",
-      "-load-plugin-executable",
-      "-in-process-plugin-server-path",
-  };
-
-  // Remove macro search path option and its argument.
-  for (auto it = cmd.begin(); it != cmd.end();) {
-    if (macroSearchOptions.contains(*it) && it + 1 != cmd.end())
-      it = cmd.erase(it, it + 2);
-    else
-      ++it;
-  }
-}
-
 class ExplicitModuleDependencyResolver {
 public:
   ExplicitModuleDependencyResolver(
@@ -302,6 +283,13 @@ class ExplicitModuleDependencyResolver {
       depInfo.addMacroDependency(
           macro.first(), macro.second.LibraryPath, macro.second.ExecutablePath);
 
+    for (auto &macro : depInfo.getMacroDependencies()) {
+      std::string arg = macro.second.LibraryPath + "#" +
+                        macro.second.ExecutablePath + "#" + macro.first;
+      commandline.push_back("-load-resolved-plugin");
+      commandline.push_back(arg);
+    }
+
     // Update CAS dependencies.
     if (auto err = collectCASDependencies(depInfo))
       return err;
@@ -529,11 +517,6 @@ class ExplicitModuleDependencyResolver {
     // Update build command line.
     if (resolvingDepInfo.isSwiftInterfaceModule() ||
         resolvingDepInfo.isSwiftSourceModule()) {
-      // If there are no external macro dependencies, drop all plugin search
-      // paths.
-      if (resolvingDepInfo.getMacroDependencies().empty() && macros.empty())
-        removeMacroSearchPaths(commandline);
-
       // Update with casfs option.
       for (auto rootID : rootIDs) {
         commandline.push_back("-cas-fs");

lib/Frontend/CompilerInvocation.cpp

@@ -2172,6 +2172,27 @@ static bool ParseSearchPathArgs(SearchPathOptions &Opts, ArgList &Args,
           resolveSearchPath(dylibPath), resolveSearchPath(serverPath)});
       break;
     }
+    case OPT_load_resolved_plugin: {
+      StringRef libraryPath;
+      StringRef executablePath;
+      StringRef modulesStr;
+      std::tie(libraryPath, executablePath) =
+          StringRef(A->getValue()).split('#');
+      std::tie(executablePath, modulesStr) = executablePath.split('#');
+      if (modulesStr.empty() ||
+          (libraryPath.empty() && executablePath.empty())) {
+        Diags.diagnose(SourceLoc(), diag::error_load_resolved_plugin,
+                       A->getValue());
+      }
+      std::vector<std::string> moduleNames;
+      for (auto name : llvm::split(modulesStr, ',')) {
+        moduleNames.emplace_back(name);
+      }
+      Opts.PluginSearchOpts.emplace_back(
+          PluginSearchOption::ResolvedPluginConfig{
+              libraryPath.str(), executablePath.str(), std::move(moduleNames)});
+      break;
+    }
     default:
       llvm_unreachable("unhandled plugin search option");
     }

lib/Frontend/ModuleInterfaceLoader.cpp

@@ -1723,38 +1723,9 @@ void InterfaceSubContextDelegateImpl::inheritOptionsForBuildingInterface(
     genericSubInvocation.setPlatformAvailabilityInheritanceMapPath(*SearchPathOpts.PlatformAvailabilityInheritanceMapPath);
   }
 
-  for (auto &entry : SearchPathOpts.PluginSearchOpts) {
-    switch (entry.getKind()) {
-    case PluginSearchOption::Kind::LoadPluginLibrary: {
-      auto &val = entry.get<PluginSearchOption::LoadPluginLibrary>();
-      GenericArgs.push_back("-load-plugin-library");
-      GenericArgs.push_back(ArgSaver.save(val.LibraryPath));
-      break;
-    }
-    case PluginSearchOption::Kind::LoadPluginExecutable: {
-      auto &val = entry.get<PluginSearchOption::LoadPluginExecutable>();
-      for (auto &moduleName : val.ModuleNames) {
-        GenericArgs.push_back("-load-plugin-executable");
-        GenericArgs.push_back(
-            ArgSaver.save(val.ExecutablePath + "#" + moduleName));
-      }
-      break;
-    }
-    case PluginSearchOption::Kind::PluginPath: {
-      auto &val = entry.get<PluginSearchOption::PluginPath>();
-      GenericArgs.push_back("-plugin-path");
-      GenericArgs.push_back(ArgSaver.save(val.SearchPath));
-      break;
-    }
-    case PluginSearchOption::Kind::ExternalPluginPath: {
-      auto &val = entry.get<PluginSearchOption::ExternalPluginPath>();
-      GenericArgs.push_back("-external-plugin-path");
-      GenericArgs.push_back(
-          ArgSaver.save(val.SearchPath + "#" + val.ServerPath));
-      break;
-    }
-    }
-  }
+  // Inherit the plugin search opts but do not inherit the arguments.
+  genericSubInvocation.getSearchPathOptions().PluginSearchOpts =
+      SearchPathOpts.PluginSearchOpts;
 
   genericSubInvocation.getFrontendOptions().InputMode
       = FrontendOptions::ParseInputMode::SwiftModuleInterface;

lib/Serialization/ModuleFileSharedCore.cpp

@@ -148,6 +148,9 @@ static bool readOptionsBlock(llvm::BitstreamCursor &cursor,
       case PluginSearchOptionKind::LoadPluginExecutable:
         optKind = PluginSearchOption::Kind::LoadPluginExecutable;
         break;
+      case PluginSearchOptionKind::ResolvedPluginConfig:
+        optKind = PluginSearchOption::Kind::ResolvedPluginConfig;
+        break;
       }
       extendedInfo.addPluginSearchOption({optKind, blobData});
       break;

lib/Serialization/ModuleFormat.h

@@ -694,6 +694,7 @@ enum class PluginSearchOptionKind : uint8_t {
   ExternalPluginPath,
   LoadPluginLibrary,
   LoadPluginExecutable,
+  ResolvedPluginConfig,
 };
 using PluginSearchOptionKindField = BCFixed<3>;
 

lib/Serialization/Serialization.cpp

@@ -1245,6 +1245,18 @@ void Serializer::writeHeader() {
                 uint8_t(PluginSearchOptionKind::LoadPluginExecutable), optStr);
             continue;
           }
+          case PluginSearchOption::Kind::ResolvedPluginConfig: {
+            auto &opt = elem.get<PluginSearchOption::ResolvedPluginConfig>();
+            std::string optStr =
+                opt.LibraryPath + "#" + opt.ExecutablePath + "#";
+            llvm::interleave(
+                opt.ModuleNames, [&](auto &name) { optStr += name; },
+                [&]() { optStr += ","; });
+            PluginSearchOpt.emit(
+                ScratchRecord,
+                uint8_t(PluginSearchOptionKind::ResolvedPluginConfig), optStr);
+            continue;
+          }
           }
         }
       }

test/CAS/macro_deps.swift

@@ -44,6 +44,10 @@
 // RUN: %{python} %S/Inputs/GenerateExplicitModuleMap.py %t/deps.json > %t/map.json
 // RUN: llvm-cas --cas %t/cas --make-blob --data %t/map.json > %t/map.casid
 
+// RUN: %FileCheck %s --check-prefix=PLUGIN_SEARCH --check-prefix=RESOLVED --input-file=%t/MyApp.cmd
+// PLUGIN_SEARCH-NOT: -external-plugin-path
+// RESOLVED-COUNT-2: -load-resolved-plugin
+
 // RUN: %target-swift-frontend -diagnostic-style=swift \
 // RUN:   -emit-module -o %t/Test.swiftmodule -cache-compile-job -cas-path %t/cas \
 // RUN:   -swift-version 5 -disable-implicit-swift-modules \

test/Macros/macro_plugin_server.swift

@@ -32,6 +32,26 @@
 
 // RUN: %FileCheck -strict-whitespace %s < %t/macro-expansions.txt
 
+/// Create file with matching name in alt directories and test
+/// `-load-resolved-plugin` takes the priority over other options.
+// RUN: %empty-directory(%t/alt)
+// RUN: touch %t/alt/%target-library-name(MacroDefinition)
+
+// RUN: env SWIFT_DUMP_PLUGIN_MESSAGING=1 %target-swift-frontend \
+// RUN:   -typecheck -verify \
+// RUN:   -swift-version 5 -enable-experimental-feature Macros \
+// RUN:   -external-plugin-path %t/alt#%swift-plugin-server \
+// RUN:   -load-resolved-plugin lib-do-not-exist.dylib##MacroDefinition \
+// RUN:   -load-resolved-plugin %t/plugins/%target-library-name(MacroDefinition)#%swift-plugin-server#MacroDefinition \
+// RUN:   -load-resolved-plugin %t/plugins/%target-library-name(EvilMacros)#%swift-plugin-server#EvilMacros \
+// RUN:   -external-plugin-path %t/alt#%swift-plugin-server \
+// RUN:   -Rmacro-loading -verify-ignore-unknown \
+// RUN:   -module-name MyApp \
+// RUN:   %s \
+// RUN:   2>&1 | tee %t/macro-expansions-2.txt
+
+// RUN: %FileCheck -strict-whitespace %s < %t/macro-expansions-2.txt
+
 // RUN: not %target-swift-frontend \
 // RUN:   -typecheck \
 // RUN:   -swift-version 5 \

tools/lldb-moduleimport-test/lldb-moduleimport-test.cpp

@@ -105,6 +105,9 @@ static bool validateModule(
       case swift::PluginSearchOption::Kind::LoadPluginExecutable:
         optStr = "-load-plugin-executable";
         break;
+      case swift::PluginSearchOption::Kind::ResolvedPluginConfig:
+        optStr = "-load-resolved-plugin";
+        break;
       }
       llvm::outs() << "    " << optStr << " " << opt.second << "\n";
     }