Merge pull request #65978 from rintaro/macros-plugin-searchorder-rdar109163929

[Macros] Rearrange plugin search order

Author: rintaro

include/swift/AST/ASTContext.h

@@ -1501,39 +1501,6 @@ class ASTContext final {
   /// Get the plugin loader.
   PluginLoader &getPluginLoader();
 
-  /// Lookup a library plugin that can handle \p moduleName and return the path
-  /// to it.
-  /// The path is valid within the VFS, use `FS.getRealPath()` for the
-  /// underlying path.
-  Optional<std::string> lookupLibraryPluginByModuleName(Identifier moduleName);
-
-  /// Load the specified dylib plugin path resolving the path with the
-  /// current VFS. If it fails to load the plugin, a diagnostic is emitted, and
-  /// returns a nullptr.
-  /// NOTE: This method is idempotent. If the plugin is already loaded, the same
-  /// instance is simply returned.
-  LoadedLibraryPlugin *loadLibraryPlugin(StringRef path);
-
-  /// Lookup an executable plugin that is declared to handle \p moduleName
-  /// module by '-load-plugin-executable'.
-  /// The path is valid within the VFS, use `FS.getRealPath()` for the
-  /// underlying path.
-  Optional<StringRef> lookupExecutablePluginByModuleName(Identifier moduleName);
-
-  /// Look for dynamic libraries in paths from `-external-plugin-path` and
-  /// return a pair of `(library path, plugin server executable)` if found.
-  /// These paths are valid within the VFS, use `FS.getRealPath()` for their
-  /// underlying path.
-  Optional<std::pair<std::string, std::string>>
-  lookupExternalLibraryPluginByModuleName(Identifier moduleName);
-
-  /// Launch the specified executable plugin path resolving the path with the
-  /// current VFS. If it fails to load the plugin, a diagnostic is emitted, and
-  /// returns a nullptr.
-  /// NOTE: This method is idempotent. If the plugin is already loaded, the same
-  /// instance is simply returned.
-  LoadedExecutablePlugin *loadExecutablePlugin(StringRef path);
-
   /// Get the output backend. The output backend needs to be initialized via
   /// constructor or `setOutputBackend`.
   llvm::vfs::OutputBackend &getOutputBackend() const {

include/swift/AST/PluginLoader.h

@@ -53,10 +53,16 @@ class PluginLoader {
   PluginRegistry *getRegistry();
 
   /// Lookup a library plugin that can handle \p moduleName and return the path
-  /// to it from `-plugin-path` or `-load-plugin-library`.
+  /// to it from `-load-plugin-library`.
   /// The path returned can be loaded by 'loadLibraryPlugin' method.
   llvm::Optional<std::string>
-  lookupLibraryPluginByModuleName(Identifier moduleName);
+  lookupExplicitLibraryPluginByModuleName(Identifier moduleName);
+
+  /// Lookup a library plugin that can handle \p moduleName and return the path
+  /// to it from `-plugin-path`.
+  /// The path returned can be loaded by 'loadLibraryPlugin' method.
+  llvm::Optional<std::string>
+  lookupLibraryPluginInSearchPathByModuleName(Identifier moduleName);
 
   /// Lookup an executable plugin that is declared to handle \p moduleName
   /// module by '-load-plugin-executable'.

lib/AST/ASTContext.cpp

@@ -6269,29 +6269,9 @@ void ASTContext::setPluginLoader(std::unique_ptr<PluginLoader> loader) {
   getImpl().Plugins = std::move(loader);
 }
 
-PluginLoader &ASTContext::getPluginLoader() { return *getImpl().Plugins; }
-
-Optional<std::string>
-ASTContext::lookupLibraryPluginByModuleName(Identifier moduleName) {
-  return getImpl().Plugins->lookupLibraryPluginByModuleName(moduleName);
-}
-
-Optional<StringRef>
-ASTContext::lookupExecutablePluginByModuleName(Identifier moduleName) {
-  return getImpl().Plugins->lookupExecutablePluginByModuleName(moduleName);
-}
-
-Optional<std::pair<std::string, std::string>>
-ASTContext::lookupExternalLibraryPluginByModuleName(Identifier moduleName) {
-  return getImpl().Plugins->lookupExternalLibraryPluginByModuleName(moduleName);
-}
-
-LoadedLibraryPlugin *ASTContext::loadLibraryPlugin(StringRef path) {
-  return getImpl().Plugins->loadLibraryPlugin(path);
-}
-
-LoadedExecutablePlugin *ASTContext::loadExecutablePlugin(StringRef path) {
-  return getImpl().Plugins->loadExecutablePlugin(path);
+PluginLoader &ASTContext::getPluginLoader() {
+  assert(getImpl().Plugins && "PluginLoader must be setup before using");
+  return *getImpl().Plugins;
 }
 
 Type ASTContext::getNamedSwiftType(ModuleDecl *module, StringRef name) {

lib/AST/PluginLoader.cpp

@@ -47,9 +47,7 @@ PluginRegistry *PluginLoader::getRegistry() {
 }
 
 llvm::Optional<std::string>
-PluginLoader::lookupLibraryPluginByModuleName(Identifier moduleName) {
-  auto fs = Ctx.SourceMgr.getFileSystem();
-
+PluginLoader::lookupExplicitLibraryPluginByModuleName(Identifier moduleName) {
   // Look for 'lib${module name}(.dylib|.so)'.
   SmallString<64> expectedBasename;
   expectedBasename.append("lib");
@@ -63,8 +61,20 @@ PluginLoader::lookupLibraryPluginByModuleName(Identifier moduleName) {
       return libPath;
     }
   }
+  return None;
+}
+
+llvm::Optional<std::string>
+PluginLoader::lookupLibraryPluginInSearchPathByModuleName(
+    Identifier moduleName) {
+  // Look for 'lib${module name}(.dylib|.so)'.
+  SmallString<64> expectedBasename;
+  expectedBasename.append("lib");
+  expectedBasename.append(moduleName.str());
+  expectedBasename.append(LTDL_SHLIB_EXT);
 
   // Try '-plugin-path'.
+  auto fs = Ctx.SourceMgr.getFileSystem();
   for (const auto &searchPath : Ctx.SearchPathOpts.PluginSearchPaths) {
     SmallString<128> fullPath(searchPath);
     llvm::sys::path::append(fullPath, expectedBasename);

lib/Sema/TypeCheckMacros.cpp

@@ -15,6 +15,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "TypeCheckMacros.h"
+#include "../AST/InlinableText.h"
 #include "TypeChecker.h"
 #include "swift/ABI/MetadataValues.h"
 #include "swift/AST/ASTContext.h"
@@ -23,9 +24,9 @@
 #include "swift/AST/CASTBridging.h"
 #include "swift/AST/DiagnosticsFrontend.h"
 #include "swift/AST/Expr.h"
-#include "../AST/InlinableText.h"
 #include "swift/AST/MacroDefinition.h"
 #include "swift/AST/NameLookupRequests.h"
+#include "swift/AST/PluginLoader.h"
 #include "swift/AST/PluginRegistry.h"
 #include "swift/AST/PrettyStackTrace.h"
 #include "swift/AST/SourceFile.h"
@@ -272,42 +273,10 @@ MacroDefinition MacroDefinitionRequest::evaluate(
 #endif
 }
 
-/// Load a plugin library based on a module name.
-static LoadedLibraryPlugin *loadLibraryPluginByName(ASTContext &ctx,
-                                                    Identifier moduleName) {
-  std::string libraryPath;
-  if (auto found = ctx.lookupLibraryPluginByModuleName(moduleName)) {
-    libraryPath = *found;
-  } else {
-    return nullptr;
-  }
-
-  // Load the plugin.
-  return ctx.loadLibraryPlugin(libraryPath);
-}
-
 static LoadedExecutablePlugin *
-loadExecutablePluginByName(ASTContext &ctx, Identifier moduleName) {
-  // Find an executable plugin.
-  std::string libraryPath;
-  std::string executablePluginPath;
-
-  if (auto found = ctx.lookupExternalLibraryPluginByModuleName(moduleName)) {
-    // Found in '-external-plugin-path'.
-    std::tie(libraryPath, executablePluginPath) = found.value();
-  } else if (auto found = ctx.lookupExecutablePluginByModuleName(moduleName)) {
-    // Found in '-load-plugin-executable'.
-    executablePluginPath = found->str();
-  }
-  if (executablePluginPath.empty())
-    return nullptr;
-
-  // Launch the plugin.
-  LoadedExecutablePlugin *executablePlugin =
-      ctx.loadExecutablePlugin(executablePluginPath);
-  if (!executablePlugin)
-    return nullptr;
-
+initializeExecutablePlugin(ASTContext &ctx,
+                           LoadedExecutablePlugin *executablePlugin,
+                           StringRef libraryPath, Identifier moduleName) {
   // Lock the plugin while initializing.
   // Note that'executablePlugn' can be shared between multiple ASTContext.
   executablePlugin->lock();
@@ -333,7 +302,7 @@ loadExecutablePluginByName(ASTContext &ctx, Identifier moduleName) {
     auto fs = ctx.SourceMgr.getFileSystem();
     if (auto err = fs->getRealPath(libraryPath, resolvedLibraryPath)) {
       ctx.Diags.diagnose(SourceLoc(), diag::compiler_plugin_not_loaded,
-                         executablePluginPath, err.message());
+                         executablePlugin->getExecutablePath(), err.message());
       return nullptr;
     }
     std::string resolvedLibraryPathStr(resolvedLibraryPath);
@@ -369,16 +338,41 @@ loadExecutablePluginByName(ASTContext &ctx, Identifier moduleName) {
 LoadedCompilerPlugin
 CompilerPluginLoadRequest::evaluate(Evaluator &evaluator, ASTContext *ctx,
                                     Identifier moduleName) const {
-  // Check dynamic link library plugins.
-  // i.e. '-plugin-path', and '-load-plugin-library'.
-  if (auto found = loadLibraryPluginByName(*ctx, moduleName)) {
-    return found;
+  PluginLoader &loader = ctx->getPluginLoader();
+
+  std::string libraryPath;
+  std::string executablePath;
+
+  // '-load-plugin-libarary'.
+  if (auto found = loader.lookupExplicitLibraryPluginByModuleName(moduleName)) {
+    libraryPath = found.value();
+  }
+  // '-load-plugin-executable'.
+  else if (auto found = loader.lookupExecutablePluginByModuleName(moduleName)) {
+    executablePath = found->str();
+  }
+  // '-plugin-path'.
+  else if (auto found =
+               loader.lookupLibraryPluginInSearchPathByModuleName(moduleName)) {
+    libraryPath = found.value();
+  }
+  // '-external-plugin-path'.
+  else if (auto found =
+               loader.lookupExternalLibraryPluginByModuleName(moduleName)) {
+    std::tie(libraryPath, executablePath) = found.value();
   }
 
-  // Fall back to executable plugins.
-  // i.e. '-external-plugin-path', and '-load-plugin-executable'.
-  if (auto *found = loadExecutablePluginByName(*ctx, moduleName)) {
-    return found;
+  if (!executablePath.empty()) {
+    if (LoadedExecutablePlugin *executablePlugin =
+            loader.loadExecutablePlugin(executablePath)) {
+      return initializeExecutablePlugin(*ctx, executablePlugin, libraryPath,
+                                        moduleName);
+    }
+  } else if (!libraryPath.empty()) {
+    if (LoadedLibraryPlugin *libraryPlugin =
+            loader.loadLibraryPlugin(libraryPath)) {
+      return libraryPlugin;
+    }
   }
 
   return nullptr;

test/Macros/macro_plugin_searchorder.swift

@@ -0,0 +1,132 @@
+
+// REQUIRES: swift_swift_parser
+
+// RUN: %empty-directory(%t)
+
+// RUN: mkdir -p %t/src
+// RUN: mkdir -p %t/bin
+// RUN: mkdir -p %t/lib/tmp
+// RUN: mkdir -p %t/lib/plugins
+// RUN: mkdir -p %t/external
+// RUN: mkdir -p %t/libexec
+
+// RUN: split-file %s %t/src
+
+//#-- For -plugin-path
+// RUN: %host-build-swift \
+// RUN:   -swift-version 5 \
+// RUN:   -emit-library -o %t/lib/plugins/%target-library-name(MacroDefinition) \
+// RUN:   -module-name MacroDefinition \
+// RUN:   -D PLUGIN_PATH \
+// RUN:   %t/src/MacroDefinition.swift \
+// RUN:   -g -no-toolchain-stdlib-rpath
+
+//#-- For -load-plugin-library
+// RUN: %host-build-swift \
+// RUN:   -swift-version 5 \
+// RUN:   -emit-library -o %t/lib/tmp/%target-library-name(MacroDefinition) \
+// RUN:   -module-name MacroDefinition \
+// RUN:   -D LOAD_PLUGIN_LIBRARY \
+// RUN:   %t/src/MacroDefinition.swift \
+// RUN:   -g -no-toolchain-stdlib-rpath
+
+//#-- For -external-plugin-path
+// RUN: %host-build-swift \
+// RUN:   -swift-version 5 \
+// RUN:   -emit-library -o %t/external/%target-library-name(MacroDefinition) \
+// RUN:   -module-name MacroDefinition \
+// RUN:   -D EXTERNAL_PLUGIN_PATH \
+// RUN:   %t/src/MacroDefinition.swift \
+// RUN:   -g -no-toolchain-stdlib-rpath
+
+//#-- For -load-plugin-executable
+// RUN: %clang \
+// RUN:  -isysroot %host_sdk \
+// RUN:  -I %swift_src_root/include \
+// RUN:  -L %swift-lib-dir -l_swiftMockPlugin \
+// RUN:  -Wl,-rpath,%swift-lib-dir \
+// RUN:  -o %t/libexec/MacroDefinitionPlugin \
+// RUN:  %t/src/MacroDefinition.c
+
+//#-- Expect -load-plugin-library
+// RUN: %target-build-swift %t/src/test.swift \
+// RUN:   -o %t/main1 \
+// RUN:   -module-name test \
+// RUN:   -plugin-path %t/lib/plugins \
+// RUN:   -external-plugin-path %t/external#%swift-plugin-server \
+// RUN:   -load-plugin-library %t/lib/tmp/%target-library-name(MacroDefinition) \
+// RUN:   -load-plugin-executable  %t/libexec/MacroDefinitionPlugin#MacroDefinition
+// RUN: %target-codesign %t/main1
+// RUN: %target-run %t/main1 | %FileCheck --check-prefix=CHECK_LOAD_PLUGIN_LIBRARY %s
+
+//#-- Expect -load-plugin-executable
+// RUN: %target-build-swift %t/src/test.swift \
+// RUN:   -o %t/main2 \
+// RUN:   -module-name test \
+// RUN:   -plugin-path %t/lib/plugins \
+// RUN:   -external-plugin-path %t/external#%swift-plugin-server \
+// RUN:   -load-plugin-executable  %t/libexec/MacroDefinitionPlugin#MacroDefinition
+// RUN: %target-codesign %t/main2
+// RUN: %target-run %t/main2 | %FileCheck --check-prefix=CHECK_LOAD_PLUGIN_EXECUTABLE %s
+
+//#-- Expect -plugin-path
+// RUN: %target-build-swift %t/src/test.swift \
+// RUN:   -o %t/main3 \
+// RUN:   -module-name test \
+// RUN:   -plugin-path %t/lib/plugins \
+// RUN:   -external-plugin-path %t/external#%swift-plugin-server
+// RUN: %target-codesign %t/main3
+// RUN: %target-run %t/main3 | %FileCheck --check-prefix=CHECK_PLUGIN_PATH %s
+
+//#-- Expect -external-plugin-path
+// RUN: %target-build-swift %t/src/test.swift \
+// RUN:   -o %t/main4 \
+// RUN:   -module-name test \
+// RUN:   -external-plugin-path %t/external#%swift-plugin-server
+// RUN: %target-codesign %t/main4
+// RUN: %target-run %t/main4 | %FileCheck --check-prefix=CHECK_EXTERNAL_PLUGIN_PATH %s
+
+// CHECK_LOAD_PLUGIN_LIBRARY: load-plugin-library
+// CHECK_LOAD_PLUGIN_EXECUTABLE: load-plugin-executable
+// CHECK_PLUGIN_PATH: plugin-path
+// CHECK_EXTERNAL_PLUGIN_PATH: external-plugin-path
+
+//--- test.swift
+@freestanding(expression) macro testMacro() -> String = #externalMacro(module: "MacroDefinition", type: "TestMacro")
+
+print(#testMacro) 
+
+//--- MacroDefinition.swift
+import SwiftSyntax
+import SwiftSyntaxMacros
+
+public struct TestMacro: ExpressionMacro {
+  public static func expansion(
+    of node: some FreestandingMacroExpansionSyntax,
+    in context: some MacroExpansionContext
+  ) throws -> ExprSyntax {
+#if PLUGIN_PATH
+    return #""plugin-path""#
+#elseif LOAD_PLUGIN_LIBRARY
+    return #""load-plugin-library""#
+#elseif EXTERNAL_PLUGIN_PATH
+    return #""external-plugin-path""#
+#endif
+  }
+}
+
+//--- MacroDefinition.c
+#include "swift-c/MockPlugin/MockPlugin.h"
+
+MOCK_PLUGIN([
+  {
+    "expect": {"getCapability": {}},
+    "response": {"getCapabilityResult": {"capability": {"protocolVersion": 1}}}
+  },
+  {
+    "expect": {"expandFreestandingMacro": {
+                "macro": {"moduleName": "MacroDefinition", "typeName": "TestMacro"},
+                "syntax": {"kind": "expression", "source": "#testMacro"}}},
+    "response": {"expandFreestandingMacroResult": {"expandedSource": "\"load-plugin-executable\"", "diagnostics": []}}
+  }
+])