[Macros] PluginRegistry vends 'LoadedLibraryPlugin' instead of 'void *'

include/swift/AST/ASTContext.h

@@ -82,6 +82,7 @@ namespace swift {
   class ExtensionDecl;
   struct ExternalSourceLocs;
   class LoadedExecutablePlugin;
+  class LoadedLibraryPlugin;
   class ForeignRepresentationInfo;
   class FuncDecl;
   class GenericContext;
@@ -1496,7 +1497,7 @@ class ASTContext final {
   /// returns a nullptr.
   /// NOTE: This method is idempotent. If the plugin is already loaded, the same
   /// instance is simply returned.
-  void *loadLibraryPlugin(StringRef path);
+  LoadedLibraryPlugin *loadLibraryPlugin(StringRef path);
 
   /// Lookup an executable plugin that is declared to handle \p moduleName
   /// module by '-load-plugin-executable'.

include/swift/AST/PluginRegistry.h

@@ -9,6 +9,8 @@
 // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
 //
 //===----------------------------------------------------------------------===//
+#ifndef SWIFT_PLUGIN_REGISTRY_H
+#define SWIFT_PLUGIN_REGISTRY_H
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringMap.h"
@@ -22,6 +24,21 @@
 
 namespace swift {
 
+/// Represent a 'dlopen'ed plugin library.
+class LoadedLibraryPlugin {
+  // Opaque handle used to interface with OS-specific dynamic library.
+  void *handle;
+
+  /// Cache of loaded symbols.
+  llvm::StringMap<void *> resolvedSymbols;
+
+public:
+  LoadedLibraryPlugin(void *handle) : handle(handle) {}
+
+  /// Finds the address of the given symbol within the library.
+  void *getAddressOfSymbol(const char *symbolName);
+};
+
 /// Represent a "resolved" exectuable plugin.
 ///
 /// Plugin clients usually deal with this object to communicate with the actual
@@ -130,7 +147,7 @@ class LoadedExecutablePlugin {
 
 class PluginRegistry {
   /// Record of loaded plugin library modules.
-  llvm::StringMap<void *> LoadedPluginLibraries;
+  llvm::StringMap<std::unique_ptr<LoadedLibraryPlugin>> LoadedPluginLibraries;
 
   /// Record of loaded plugin executables.
   llvm::StringMap<std::unique_ptr<LoadedExecutablePlugin>>
@@ -146,7 +163,7 @@ class PluginRegistry {
 
   /// Load a dynamic link library specified by \p path.
   /// If \p path plugin is already loaded, this returns the cached object.
-  llvm::Expected<void *> loadLibraryPlugin(llvm::StringRef path);
+  llvm::Expected<LoadedLibraryPlugin *> loadLibraryPlugin(llvm::StringRef path);
 
   /// Load an executable plugin specified by \p path .
   /// If \p path plugin is already loaded, this returns the cached object.
@@ -155,3 +172,5 @@ class PluginRegistry {
 };
 
 } // namespace swift
+
+#endif // SWIFT_PLUGIN_REGISTRY_H

include/swift/AST/TypeCheckRequests.h

@@ -26,6 +26,7 @@
 #include "swift/AST/Type.h"
 #include "swift/AST/Evaluator.h"
 #include "swift/AST/Pattern.h"
+#include "swift/AST/PluginRegistry.h"
 #include "swift/AST/ProtocolConformance.h"
 #include "swift/AST/SimpleRequest.h"
 #include "swift/AST/SourceFile.h"
@@ -49,7 +50,6 @@ struct ExternalMacroDefinition;
 class ClosureExpr;
 class GenericParamList;
 class LabeledStmt;
-class LoadedExecutablePlugin;
 class MacroDefinition;
 class PrecedenceGroupDecl;
 class PropertyWrapperInitializerInfo;
@@ -3998,39 +3998,20 @@ class ExpandSynthesizedMemberMacroRequest
   bool isCached() const { return true; }
 };
 
-/// Load a plugin module with the given name.
-///
-///
+/// Represent a loaded plugin either an in-process library or an executable.
 class LoadedCompilerPlugin {
-  enum class PluginKind : uint8_t {
-    None,
-    InProcess,
-    Executable,
-  };
-  PluginKind kind;
-  void *ptr;
-
-  LoadedCompilerPlugin(PluginKind kind, void *ptr) : kind(kind), ptr(ptr) {
-    assert(ptr != nullptr || kind == PluginKind::None);
-  }
+  llvm::PointerUnion<LoadedLibraryPlugin *, LoadedExecutablePlugin *> ptr;
 
 public:
-  LoadedCompilerPlugin(std::nullptr_t) : kind(PluginKind::None), ptr(nullptr) {}
-
-  static LoadedCompilerPlugin inProcess(void *ptr) {
-    return {PluginKind::InProcess, ptr};
-  }
-  static LoadedCompilerPlugin executable(LoadedExecutablePlugin *ptr) {
-    return {PluginKind::Executable, ptr};
-  }
+  LoadedCompilerPlugin(std::nullptr_t) : ptr(nullptr) {}
+  LoadedCompilerPlugin(LoadedLibraryPlugin *ptr) : ptr(ptr){};
+  LoadedCompilerPlugin(LoadedExecutablePlugin *ptr) : ptr(ptr){};
 
-  void *getAsInProcessPlugin() const {
-    return kind == PluginKind::InProcess ? ptr : nullptr;
+  LoadedLibraryPlugin *getAsLibraryPlugin() const {
+    return ptr.dyn_cast<LoadedLibraryPlugin *>();
   }
   LoadedExecutablePlugin *getAsExecutablePlugin() const {
-    return kind == PluginKind::Executable
-               ? static_cast<LoadedExecutablePlugin *>(ptr)
-               : nullptr;
+    return ptr.dyn_cast<LoadedExecutablePlugin *>();
   }
 };
 

lib/AST/ASTContext.cpp

@@ -6412,7 +6412,7 @@ LoadedExecutablePlugin *ASTContext::loadExecutablePlugin(StringRef path) {
   return plugin.get();
 }
 
-void *ASTContext::loadLibraryPlugin(StringRef path) {
+LoadedLibraryPlugin *ASTContext::loadLibraryPlugin(StringRef path) {
   // Remember the path (even if it fails to load.)
   getImpl().LoadedPluginLibraryPaths.insert(path);
 

lib/AST/PluginRegistry.cpp

@@ -44,13 +44,15 @@ PluginRegistry::PluginRegistry() {
   dumpMessaging = ::getenv("SWIFT_DUMP_PLUGIN_MESSAGING") != nullptr;
 }
 
-llvm::Expected<void *> PluginRegistry::loadLibraryPlugin(StringRef path) {
+llvm::Expected<LoadedLibraryPlugin *>
+PluginRegistry::loadLibraryPlugin(StringRef path) {
   std::lock_guard<std::mutex> lock(mtx);
-  auto found = LoadedPluginLibraries.find(path);
-  if (found != LoadedPluginLibraries.end()) {
+  auto &storage = LoadedPluginLibraries[path];
+  if (storage) {
     // Already loaded.
-    return found->second;
+    return storage.get();
   }
+
   void *lib = nullptr;
 #if defined(_WIN32)
   lib = LoadLibraryA(path.str().c_str());
@@ -64,8 +66,19 @@ llvm::Expected<void *> PluginRegistry::loadLibraryPlugin(StringRef path) {
     return llvm::createStringError(llvm::inconvertibleErrorCode(), dlerror());
   }
 #endif
-  LoadedPluginLibraries.insert({path, lib});
-  return lib;
+
+  storage = std::make_unique<LoadedLibraryPlugin>(lib);
+  return storage.get();
+}
+
+void *LoadedLibraryPlugin::getAddressOfSymbol(const char *symbolName) {
+  auto &cached = resolvedSymbols[symbolName];
+  if (cached)
+    return cached;
+#if !defined(_WIN32)
+  cached = dlsym(handle, symbolName);
+#endif
+  return cached;
 }
 
 llvm::Expected<LoadedExecutablePlugin *>
@@ -98,8 +111,8 @@ PluginRegistry::loadExecutablePlugin(StringRef path) {
                                    "not executable");
   }
 
-  auto plugin = std::unique_ptr<LoadedExecutablePlugin>(
-      new LoadedExecutablePlugin(path, stat.getLastModificationTime()));
+  auto plugin = std::make_unique<LoadedExecutablePlugin>(
+      path, stat.getLastModificationTime());
 
   plugin->setDumpMessaging(dumpMessaging);
 
@@ -140,9 +153,9 @@ llvm::Error LoadedExecutablePlugin::spawnIfNeeded() {
     return llvm::errorCodeToError(childInfo.getError());
   }
 
-  Process = std::unique_ptr<PluginProcess>(
-      new PluginProcess(childInfo->Pid, childInfo->ReadFileDescriptor,
-                        childInfo->WriteFileDescriptor));
+  Process = std::make_unique<PluginProcess>(childInfo->Pid,
+                                            childInfo->ReadFileDescriptor,
+                                            childInfo->WriteFileDescriptor);
 
   // Call "on reconnect" callbacks.
   for (auto *callback : onReconnect) {

lib/Sema/TypeCheckMacros.cpp

@@ -38,14 +38,6 @@
 #include "swift/Subsystems.h"
 #include "llvm/Config/config.h"
 
-#if defined(_WIN32)
-#define WIN32_LEAN_AND_MEAN
-#define NOMINMAX
-#include <windows.h>
-#else
-#include <dlfcn.h>
-#endif
-
 using namespace swift;
 
 extern "C" void *swift_ASTGen_resolveMacroType(const void *macroType);
@@ -90,10 +82,10 @@ extern "C" bool swift_ASTGen_pluginServerLoadLibraryPlugin(
 
 #if SWIFT_SWIFT_PARSER
 /// Look for macro's type metadata given its external module and type name.
-static void const *lookupMacroTypeMetadataByExternalName(
-    ASTContext &ctx, StringRef moduleName, StringRef typeName,
-    void *libraryHint = nullptr
-) {
+static void const *
+lookupMacroTypeMetadataByExternalName(ASTContext &ctx, StringRef moduleName,
+                                      StringRef typeName,
+                                      LoadedLibraryPlugin *plugin) {
   // Look up the type metadata accessor as a struct, enum, or class.
   const Demangle::Node::Kind typeKinds[] = {
     Demangle::Node::Kind::Structure,
@@ -105,11 +97,7 @@ static void const *lookupMacroTypeMetadataByExternalName(
   for (auto typeKind : typeKinds) {
     auto symbolName = Demangle::mangledNameForTypeMetadataAccessor(
         moduleName, typeName, typeKind);
-#if !defined(_WIN32)
-    /// FIXME: 'PluginRegistry' should vend a wrapper object of the library
-    /// handle (like llvm::sys::DynamicLibrary) and dlsym should be abstracted.
-    accessorAddr = dlsym(libraryHint, symbolName.c_str());
-#endif
+    accessorAddr = plugin->getAddressOfSymbol(symbolName.c_str());
     if (accessorAddr)
       break;
   }
@@ -276,7 +264,8 @@ MacroDefinition MacroDefinitionRequest::evaluate(
 }
 
 /// Load a plugin library based on a module name.
-static void *loadLibraryPluginByName(ASTContext &ctx, Identifier moduleName) {
+static LoadedLibraryPlugin *loadLibraryPluginByName(ASTContext &ctx,
+                                                    Identifier moduleName) {
   std::string libraryPath;
   if (auto found = ctx.lookupLibraryPluginByModuleName(moduleName)) {
     libraryPath = *found;
@@ -367,33 +356,28 @@ loadExecutablePluginByName(ASTContext &ctx, Identifier moduleName) {
 LoadedCompilerPlugin
 CompilerPluginLoadRequest::evaluate(Evaluator &evaluator, ASTContext *ctx,
                                     Identifier moduleName) const {
-  auto fs = ctx->SourceMgr.getFileSystem();
-  auto &searchPathOpts = ctx->SearchPathOpts;
-  auto *registry = ctx->getPluginRegistry();
-
   // Check dynamic link library plugins.
   // i.e. '-plugin-path', and '-load-plugin-library'.
-  if (auto found = loadLibraryPluginByName(*ctx, moduleName))
-    return LoadedCompilerPlugin::inProcess(found);
+  if (auto found = loadLibraryPluginByName(*ctx, moduleName)) {
+    return found;
+  }
 
   // Fall back to executable plugins.
   // i.e. '-external-plugin-path', and '-load-plugin-executable'.
   if (auto *found = loadExecutablePluginByName(*ctx, moduleName)) {
-    return LoadedCompilerPlugin::executable(found);
+    return found;
   }
 
   return nullptr;
 }
 
 static Optional<ExternalMacroDefinition>
-resolveInProcessMacro(
-    ASTContext &ctx, Identifier moduleName, Identifier typeName,
-    void *libraryHint = nullptr
-) {
+resolveInProcessMacro(ASTContext &ctx, Identifier moduleName,
+                      Identifier typeName, LoadedLibraryPlugin *plugin) {
 #if SWIFT_SWIFT_PARSER
   /// Look for the type metadata given the external module and type names.
   auto macroMetatype = lookupMacroTypeMetadataByExternalName(
-      ctx, moduleName.str(), typeName.str(), libraryHint);
+      ctx, moduleName.str(), typeName.str(), plugin);
   if (macroMetatype) {
     // Check whether the macro metatype is in-process.
     if (auto inProcess = swift_ASTGen_resolveMacroType(macroMetatype)) {
@@ -438,7 +422,7 @@ ExternalMacroDefinitionRequest::evaluate(Evaluator &evaluator, ASTContext *ctx,
   LoadedCompilerPlugin loaded =
       evaluateOrDefault(evaluator, loadRequest, nullptr);
 
-  if (auto loadedLibrary = loaded.getAsInProcessPlugin()) {
+  if (auto loadedLibrary = loaded.getAsLibraryPlugin()) {
     if (auto inProcess = resolveInProcessMacro(
             *ctx, moduleName, typeName, loadedLibrary))
       return *inProcess;