Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Macros] Add swift-plugin-server executable #64376

Merged
merged 6 commits into from
Mar 20, 2023

Conversation

rintaro
Copy link
Member

@rintaro rintaro commented Mar 15, 2023

(depends on swiftlang/swift-syntax#1411)

Add tools/swift-plugin-server. This executable is intended to be installed in the toolchain and act as an executable compiler plugin just like other 'macro' plugins.

This plugin server has an optional method loadPluginLibrary that dynamically loads dylib plugins.
The compiler has a newly added option -external-plugin-path. This option receives a pair of the plugin library search path (just like -plugin-path) and the corresponding "plugin server" path, separated by #. i.e.

  -external-plugin-path
    <plugin library search path>#<plugin server exectuable path>

For exmaple, when there's a macro decl:

  @freestanding(expression)
  macro stringify<T>(T) -> (T, String) =
      #externalMacro(module: "BasicMacro", type: "StringifyMacro")

The compiler look for libBasicMacro.dylib in -plugin-path paths first, if not found, it falls back to -external-plugin-path and tries to find libBasicMacro.dylib in them. If it's found, the "plugin server" path is launched just like an executable plugin, then loadPluginLibrary method is invoked via IPC, which dlopen the library path in the plugin server. At the actual macro expansion, the mangled name for BasicMacro.SinglifyMacro is used to resolve the macro just like dylib plugins in the compiler.

This is useful for

  • Isolating the plugin process, so the plugin crashes doesn't result the compiler crash
  • Being able to use library plugins linked with other swift-syntax versions

rdar://105104850

@rintaro
Copy link
Member Author

rintaro commented Mar 15, 2023

swiftlang/swift-syntax#1411
@swift-ci Please smoke test

@rintaro rintaro marked this pull request as ready for review March 15, 2023 16:12
@rintaro rintaro requested review from DougGregor and bnbarham and removed request for xedin, slavapestov, CodaFi, artemcm, hborla and zoecarver March 15, 2023 16:12
@@ -67,6 +67,7 @@
#include "llvm/ADT/Statistic.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringSwitch.h"
#include "llvm/Config/config.h"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For LTDL_SHLIB_EXT

cxxDiagnosticEngine: UnsafeMutablePointer<UInt8>
) -> Bool {
let plugin = CompilerPlugin(opaqueHandle: opaqueHandle)
assert(plugin.capability.features?.contains("loadPluginLibrary") == true)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only usage of the newly added Capability.features at this point. In case the plugin server doesn't actually implement loadPluginLibrary, the request would just fail.

@rintaro
Copy link
Member Author

rintaro commented Mar 15, 2023

swiftlang/swift-syntax#1411
@swift-ci Please smoke test

@rintaro
Copy link
Member Author

rintaro commented Mar 15, 2023

swiftlang/swift-syntax#1411
@swift-ci Please smoke test

1 similar comment
@rintaro
Copy link
Member Author

rintaro commented Mar 16, 2023

swiftlang/swift-syntax#1411
@swift-ci Please smoke test

This executable is intended to be installed in the toolchain and act as
an executable compiler plugin just like other 'macro' plugins.

This plugin server has an optional method 'loadPluginLibrary' that
dynamically loads dylib plugins.
The compiler has a newly added option '-external-plugin-path'. This
option receives a pair of the plugin library search path (just like
'-plugin-path') and the corresponding "plugin server" path, separated
by '#'. i.e.

  -external-plugin-path
    <plugin library search path>#<plugin server executable path>

For exmaple, when there's a macro decl:

  @freestanding(expression)
  macro stringify<T>(T) -> (T, String) =
      #externalMacro(module: "BasicMacro", type: "StringifyMacro")

The compiler look for 'libBasicMacro.dylib' in '-plugin-path' paths,
if not found, it falls back to '-external-plugin-path' and tries to find
'libBasicMacro.dylib' in them. If it's found, the "plugin server" path
is launched just like an executable plugin, then 'loadPluginLibrary'
method is invoked via IPC, which 'dlopen' the library path in the plugin
server. At the actual macro expansion, the mangled name for
'BasicMacro.StringifyMacro' is used to resolve the macro  just like
dylib plugins in the compiler.

This is useful for
 * Isolating the plugin process, so the plugin crashes doesn't result
   the compiler crash
 * Being able to use library plugins linked with other `swift-syntax`
   versions

rdar://105104850
…est'

Separate "load plugin" and "resolve macro" phases. So that the loaded
executable plugin is now cached in ASTContext and reused. This saves
a 'loadPluginLibrary' IPC messaging when the library provides multiple
macros.
Also, build pure swift library/tool with install RPATH like non Swift
things.
@rintaro
Copy link
Member Author

rintaro commented Mar 17, 2023

swiftlang/swift-syntax#1411
@swift-ci Please smoke test

@rintaro
Copy link
Member Author

rintaro commented Mar 17, 2023

swiftlang/swift-syntax#1411
@swift-ci Please smoke test

Comment on lines 1470 to 1472
/// 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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Type on equialent. But how about:

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.

One problem with getRealPath here is that it's possible that the overlay has useExternalName set to false, in which case that still won't give the real path 😓.

/// 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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this std::string is the # separated compiler plugin search path + path to plugin server? If so, maybe:

`#` separated strings containing the compiler plugin search path and path to plugin server.

Or, alternatively, split these when getting the options into either a pair or small custom struct (I'd prefer the latter).

Also, this split seems a little less safe than the path#name one we had previously as it is now two paths rather than a path and a name. IIRC you said that # wasn't valid on all platforms though?

Copy link
Member Author

@rintaro rintaro Mar 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will make a custom struct.

# is not a special character as a file name at least in macos, (i.e. you can touch ##.txt without errors). I just said it's not common to use # in directory name or filename, and there's no special meaning in shell, unlike ; or ?, etc.
I agree it's not safe, but...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No reason to block on this, # is just a little scary to me because of #1/#2 etc. I can definitely imagine someone doing that 😅

}

void *getAsInProcessPlugin() const {
return kind == PluginKind::InProcess ? static_cast<void *>(ptr) : nullptr;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was the static_cast here just a copy paste? Could use a PointerUnion instead to avoid the need for PluginKind.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Decided not to use PointerUnion because we don't know the alignment of void *

@@ -237,6 +237,7 @@ void ToolChain::addCommonFrontendArgs(const OutputInfo &OI,
inputArgs.AddAllArgs(arguments, options::OPT_F, options::OPT_Fsystem);
inputArgs.AddAllArgs(arguments, options::OPT_vfsoverlay);
inputArgs.AddAllArgs(arguments, options::OPT_plugin_path);
inputArgs.AddAllArgs(arguments, options::OPT_external_plugin_path);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to implement this in swift-driver too?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we need. Since it's auto generated, I want to do that using main after I merge this PR

@@ -0,0 +1,32 @@
if (SWIFT_SWIFT_PARSER)
# _swiftCSwiftPluginServer is just a C support library for wift-plugin-server
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# _swiftCSwiftPluginServer is just a C support library for wift-plugin-server
# _swiftCSwiftPluginServer is just a C support library for swift-plugin-server

@rintaro
Copy link
Member Author

rintaro commented Mar 17, 2023

@swift-ci Please smoke test

1 similar comment
@rintaro
Copy link
Member Author

rintaro commented Mar 17, 2023

@swift-ci Please smoke test

@rintaro
Copy link
Member Author

rintaro commented Mar 17, 2023

@swift-ci Please smoke test

* New struct to store a pair of plugin search path and the server path
* typo
@rintaro
Copy link
Member Author

rintaro commented Mar 18, 2023

@swift-ci Please smoke test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants