[Macros] Small review changes

* New struct to store a pair of plugin search path and the server path
* typo

Author: rintaro

include/swift/AST/ASTContext.h

@@ -1468,14 +1468,15 @@ class ASTContext final {
   Type getNamedSwiftType(ModuleDecl *module, StringRef name);
 
   /// Lookup an executable plugin that is declared to handle \p moduleName
-  /// module by '-load-plugin-executable'. Note that the returned path might be
-  /// in the current VFS. i.e. use FS.getRealPath() to get the real path.
+  /// module by '-load-plugin-executable'.
+  /// The path is valid within the VFS, use `FS.getRealPath()` for the
+  /// underlying path.
   Optional<StringRef> lookupExecutablePluginByModuleName(Identifier moduleName);
 
-  /// From paths '-external-plugin-path', look for dylib file that has
-  /// 'lib${moduleName}.dylib' (or equialent depending on the platform) and
-  /// return the found dylib path and the path to the "plugin server" for that.
-  /// Note that the returned path might be in the current VFS.
+  /// 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);
 

include/swift/AST/SearchPathOptions.h

@@ -174,6 +174,13 @@ class ModuleSearchPathLookup {
                             llvm::vfs::FileSystem *FS, bool IsOSDarwin);
 };
 
+/// Pair of a plugin search path and the corresponding plugin server executable
+/// path.
+struct ExternalPluginSearchPathAndServerPath {
+  std::string SearchPath;
+  std::string ServerPath;
+};
+
 /// Options for controlling search path behavior.
 class SearchPathOptions {
   /// To call \c addImportSearchPath and \c addFrameworkSearchPath from
@@ -382,10 +389,11 @@ class SearchPathOptions {
   /// macro implementations.
   std::vector<std::string> PluginSearchPaths;
 
-  /// Paths that contain compiler plugins and the path to the plugin server
-  /// executable.
-  /// e.g. '/path/to/usr/lib/swift/host/plugins#/path/to/usr/bin/plugin-server'.
-  std::vector<std::string> ExternalPluginSearchPaths;
+  /// Pairs of external compiler plugin search paths and the corresponding
+  /// plugin server executables.
+  /// e.g. {"/path/to/usr/lib/swift/host/plugins",
+  ///       "/path/to/usr/bin/plugin-server"}
+  std::vector<ExternalPluginSearchPathAndServerPath> ExternalPluginSearchPaths;
 
   /// Don't look in for compiler-provided modules.
   bool SkipRuntimeLibraryImportPaths = false;

include/swift/AST/TypeCheckRequests.h

@@ -4025,7 +4025,7 @@ class LoadedCompilerPlugin {
   }
 
   void *getAsInProcessPlugin() const {
-    return kind == PluginKind::InProcess ? static_cast<void *>(ptr) : nullptr;
+    return kind == PluginKind::InProcess ? ptr : nullptr;
   }
   LoadedExecutablePlugin *getAsExecutablePlugin() const {
     return kind == PluginKind::Executable

lib/AST/ASTContext.cpp

@@ -6358,15 +6358,11 @@ Optional<std::pair<std::string, std::string>>
 ASTContext::lookupExternalLibraryPluginByModuleName(Identifier moduleName) {
   auto fs = this->SourceMgr.getFileSystem();
   for (auto &pair : SearchPathOpts.ExternalPluginSearchPaths) {
-    StringRef searchPath;
-    StringRef serverPath;
-    std::tie(searchPath, serverPath) = StringRef(pair).split('#');
-
-    SmallString<128> fullPath(searchPath);
+    SmallString<128> fullPath(pair.SearchPath);
     llvm::sys::path::append(fullPath, "lib" + moduleName.str() + LTDL_SHLIB_EXT);
 
     if (fs->exists(fullPath)) {
-      return {{std::string(fullPath), serverPath.str()}};
+      return {{std::string(fullPath), pair.ServerPath}};
     }
   }
   return None;

lib/Frontend/CompilerInvocation.cpp

@@ -1494,11 +1494,12 @@ static bool ParseSearchPathArgs(SearchPathOptions &Opts,
 
   for (const Arg *A : Args.filtered(OPT_external_plugin_path)) {
     // '<plugin directory>#<plugin server executable path>'.
+    // FIXME: '#' can be used in the paths.
     StringRef dylibPath;
     StringRef serverPath;
     std::tie(dylibPath, serverPath) = StringRef(A->getValue()).split('#');
-    Opts.ExternalPluginSearchPaths.push_back(
-        resolveSearchPath(dylibPath) + "#" + resolveSearchPath(serverPath));
+    Opts.ExternalPluginSearchPaths.emplace_back(
+        resolveSearchPath(dylibPath), resolveSearchPath(serverPath));
   }
 
   for (const Arg *A : Args.filtered(OPT_L)) {

tools/swift-plugin-server/CMakeLists.txt

@@ -1,5 +1,5 @@
 if (SWIFT_SWIFT_PARSER)
-  # _swiftCSwiftPluginServer is just a C support library for  wift-plugin-server
+  # _swiftCSwiftPluginServer is just a C support library for swift-plugin-server
   # Don't bother to create '.a' for that.
   add_swift_host_library(_swiftCSwiftPluginServer OBJECT
     Sources/CSwiftPluginServer/PluginServer.cpp