[Macros] In-process plugin server #73725

rintaro · 2024-05-17T22:55:48Z

Separate swift-syntax libs for the compiler and for the library plugins. Compiler communicates with library plugins using serialized messages just like executable plugins.

lib/swift/host/compiler/lib_Compiler*.dylib(source lib/CompilerSwiftSyntax): swift-syntax libraries for compiler. Library evolution is disabled.
Compiler (ASTGen and swiftIDEUtilsBridging) only depends on lib/swift/host/compiler libraries.
lib/swift/host/libSwiftInProcPluginServer.dylib (source tools/swift-plugin-server/Sources/SwiftInProcPluginServer): In-process plugin server shared library. This has one swift_inproc_plugins_handle_message entry point that receives a message and return the response.
In the compiler:
- Add -in-process-plugin-server-path front-end option, which specifies the SwiftInProcPluginServer shared library path.
- Remove LoadedLibraryPlugin, because all library plugins are managed by SwiftInProcPluginServer
- Introduce abstract CompilerPlugin class that has 2 subclasses:
  - LoadedExecutablePlugin existing class that represents an executable plugin
  - InProcessPlugins wraps dlopened SwiftInProcPluginServer and communicate with it via the swift_inproc_plugins_handle_message
- Unified the code path in TypeCheckMacros.cpp and ASTGen, the difference between executable plugins and library plugins are now abstracted by CompilerPlugin

rintaro · 2024-05-17T23:01:26Z

lib/CompilerSwiftSyntax/CMakeLists.txt

+# Make unified swift-syntax shared library.
+add_pure_swift_host_library(swiftSyntaxUnified SHARED)
+set_target_properties(swiftSyntaxUnified PROPERTIES LINKER_LANGUAGE Swift)
+
+macro(target_link_libraries_whole_archive target)
+ foreach(lib ${ARGN}) 
+ target_compile_options(${lib} PRIVATE "SHELL:-module-link-name ${target}")
+ target_sources(${target} PRIVATE $<TARGET_OBJECTS:${lib}>)
+ # Workaround for CMake doesn't recognize '.o' files as sources.
+ target_link_options(${target} PRIVATE $<TARGET_OBJECTS:${lib}>)
+ set_property(TARGET ${target} APPEND PROPERTY
+ INTERFACE_INCLUDE_DIRECTORIES "$<TARGET_PROPERTY:${lib},INTERFACE_INCLUDE_DIRECTORIES>"
+ )


Instead of making an aggregated swift-syntax libraries, can we link those static libraries with ASTGen and make libASTGen.dylib?

edit: no because ASTGen depends on C++ code.

rintaro · 2024-05-17T23:03:59Z

lib/ASTGen/Sources/ASTGen/PluginHost.swift

@@ -319,7 +319,7 @@ class PluginDiagnosticsEngine {
 messageSuffix: String? = nil
 ) {
 for diagnostic in diagnostics {
- self.emit(diagnostic)
+ self.emit(diagnostic, messageSuffix: messageSuffix)


We had a bug in executable plugins where (from macro '\(macroName)') suffix wasn't added in the diagnostics from executable plugins 😞

hamishknight · 2024-05-22T16:40:15Z

include/swift/AST/PluginRegistry.h

- llvm::StringMap<void *> resolvedSymbols;
+ std::mutex mtx;
+
+ /// Opaque value of the protocol capability of the pluugin. This is a


Suggested change

/// Opaque value of the protocol capability of the pluugin. This is a

/// Opaque value of the protocol capability of the plugin. This is a

hamishknight · 2024-05-22T16:55:51Z

lib/AST/PluginRegistry.cpp

- // Already loaded.
- return storage.get();
+CompilerPlugin::~CompilerPlugin() {
+ // Let ASTGen to cleanup things.


Suggested change

// Let ASTGen to cleanup things.

// Let ASTGen do cleanup things.

hamishknight · 2024-05-22T17:35:32Z

lib/AST/PluginRegistry.cpp

+ auto message = receivedResponse;
+ receivedResponse = "";


Could get rid of the string copy here, but probably doesn't matter that much

👍

SWIFT_DEFER { receivedResponse = ""; }; return std::move(receivedResponse);

hamishknight · 2024-05-22T17:40:26Z

lib/AST/PluginRegistry.cpp

+llvm::Expected<InProcessPlugins *>
+InProcessPlugins::create(const char *serverPath) {


Any reason not to have this return a unique_ptr?

No. Changed to return unique_ptr 👍

hamishknight · 2024-05-22T17:53:11Z

lib/ASTGen/Sources/ASTGen/PluginHost.swift

+ var basename: String {
+ guard
+ let lastSlash = lastIndex(where: {
+ #if os(iOS) || os(macOS) || os(tvOS) || os(watchOS) || os(Android) || os(Linux)


visionOS?

hamishknight · 2024-05-22T18:06:11Z

tools/libSwiftScan/CMakeLists.txt

@@ -46,20 +46,20 @@ endif()

 if(SWIFT_BUILD_SWIFT_SYNTAX)
 if(SWIFT_HOST_VARIANT_SDK IN_LIST SWIFT_DARWIN_PLATFORMS)
- # Ensure that we can find the host shared libraries.
+ # Ensure that we can find the shaered swiftsyntax libraries.


Suggested change

# Ensure that we can find the shaered swiftsyntax libraries.

# Ensure that we can find the shared swiftsyntax libraries.

rintaro · 2024-05-24T04:31:12Z

cmake/modules/AddPureSwift.cmake

- Swift_MODULE_NAME ${name}
- Swift_MODULE_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})
- set(module_file "${CMAKE_CURRENT_BINARY_DIR}/${name}.swiftmodule")
+ set(module_dir "${CMAKE_CURRENT_BINARY_DIR}/modules")


Fragile modules are now created in ${CMAKE_CURRENT_BINARY_DIR}/modules. This is to avoid PR testing failures in incremental bots. This PR changes _CompilerRegexParser to fragile, if we create ${CMAKE_CURRENT_BINARY_DIR}/_CompilerRegexParser.swiftmodule , subsequent PR testings will be confused because -I ${CMAKE_CURRENT_BINARY_DIR} is added by CMake. Pre-patch #73799 can avoid that, but just in case.

rintaro · 2024-05-24T04:37:02Z

test/Macros/macro_expand.swift

@@ -629,8 +629,8 @@ struct HasNestedType {
 #if TEST_DIAGNOSTICS
 @freestanding(expression)
 macro missingMacro() = #externalMacro(module: "MacroDefinition", type: "BluhBlah")
-// expected-warning@-1 {{external macro implementation type 'MacroDefinition.BluhBlah' could not be found for macro 'missingMacro()'; 'MacroDefinition.BluhBlah' could not be found in library plugin '}}
+// FIXME: xpected-warning@-1 {{external macro implementation type 'MacroDefinition.BluhBlah' could not be found for macro 'missingMacro()'; 'MacroDefinition.BluhBlah' could not be found in library plugin '}}


This is because the plugin message facilities (i.e. executable plugins) doesn't support "resolve macro type" at this point. Expansions would just fail and be diagnosed if the type is not found or not a macro type.

Maybe we can introduce case resolveMacroType(module: String, type: String) plugin message to recover this.

tools/libSwiftScan/CMakeLists.txt

rintaro · 2024-05-24T16:49:55Z

apple/swift-syntax#2659
apple/swift-driver#1616
@swift-ci Please smoke test

rintaro · 2024-05-24T21:41:19Z

apple/swift-syntax#2659
apple/swift-driver#1616
@swift-ci Please smoke test

rintaro · 2024-05-24T23:20:47Z

apple/swift-syntax#2659
apple/swift-driver#1616
@swift-ci Please smoke test

rintaro · 2024-05-25T00:04:31Z

apple/swift-syntax#2659
apple/swift-driver#1616
@swift-ci Please smoke test

rintaro · 2024-05-25T00:51:59Z

apple/swift-syntax#2659
apple/swift-driver#1616
@swift-ci Please smoke test

rintaro · 2024-05-25T02:31:24Z

apple/swift-syntax#2659
apple/swift-driver#1616
@swift-ci Please smoke test

rintaro · 2024-05-25T03:57:57Z

apple/swift-syntax#2659
apple/swift-driver#1616
@swift-ci Please smoke test Windows

rintaro · 2024-05-25T06:27:27Z

apple/swift-syntax#2659
apple/swift-driver#1616
@swift-ci Please smoke test Windows

kabiroberai · 2024-05-29T02:15:26Z

👋 does this PR remove support for the loadPluginLibrary approach entirely? Worth noting that we're relying on this mechanism in #73031 in order to enable support for WebAssembly-based plugins. We could move these in-process but I think it would be preferable to keep third-party Wasm plugins out-of-process for defence-in-depth/isolation reasons (especially if we eventually want to execute them with JIT), so IMO it's worth keeping the library plugin mechanism for that. Also, IIUC one of the ways we could improve macro performance is to reuse a single plugin library/executable across swiftc invocations. Would this PR preclude that change for library-based plugins?

I should mention that I don't fully understand the implications of this PR so please do correct me if I'm mistaken. In general, in-process execution for first-party plugins during one-off swiftc invocation does seem like a nice improvement!

rintaro · 2024-05-30T18:56:46Z

does this PR remove support for the loadPluginLibrary approach entirely?

I'm not 100% sure what "loadPluginLibrary approach" you mean, but either it's about -load-plugin-library compiler option or loadPluginLibrary plugin message, this PR doesn't remove them.

Since we need to keep supporting "in-process" plugins (i.e. -load-plugin-library and -plugin-path compiler options) for various reasons, we've been suffered by some limitations from that. The main intent of this PR is that separating swift-syntax libraries for the compiler and swift-syntax libraries for those "in-process" plugins. By that

We can unify the code path between in-process and out-of-process plugins
We can use different swift-syntax between the compiler and the in-process plugins. That let us compiling swift-syntax for the compiler without library-evolution, for the compiler performance

This PR doesn't have any intent of removing or deprecating out-of-process plugin mechanism.

IIUC one of the ways we could improve macro performance is to reuse a single plugin library/executable across swiftc invocations. Would this PR preclude that change for library-based plugins?

Yes, reusing plugin processes across multiple swift-frontend invocations is probably what we need to tackle on next. This PR does not preclude that approach.

I'm not sure I entirely understand your concerns. Let me know if you still have any questions!

rintaro · 2024-05-30T20:23:10Z

apple/swift-syntax#2659
apple/swift-driver#1616
@swift-ci Please smoke test

rintaro · 2024-05-30T22:04:52Z

apple/swift-syntax#2659
apple/swift-driver#1616
@swift-ci Please smoke test

kabiroberai · 2024-05-31T00:39:55Z

Ah I was under the impression that this removed support for external plugin servers in favour of in-process plugins, but looks like I was confused. Thanks for the clarification!

rintaro · 2024-05-31T21:01:04Z

apple/swift-syntax#2659
apple/swift-driver#1616
@swift-ci Please smoke test

rintaro · 2024-05-31T22:01:26Z

apple/swift-syntax#2659
apple/swift-driver#1616
@swift-ci Please smoke test Linux

rintaro · 2024-06-04T18:16:31Z

apple/swift-syntax#2659
apple/swift-driver#1616
@swift-ci Please smoke test Linux

rintaro · 2024-06-04T19:43:56Z

apple/swift-syntax#2659
apple/swift-driver#1616
@swift-ci Please smoke test

rintaro · 2024-06-04T22:08:00Z

apple/swift-syntax#2659
apple/swift-driver#1616
@swift-ci Please smoke test

rintaro · 2024-06-05T21:52:17Z

apple/swift-syntax#2659
apple/swift-driver#1616
@swift-ci Please smoke test

rintaro · 2024-06-05T22:03:57Z

apple/llvm-project#8855
apple/swift-syntax#2659
apple/swift-driver#1616
@swift-ci Please smoke test

Separate swift-syntax libs for the compiler and for the library plugins. Compiler communicates with library plugins using serialized messages just like executable plugins. * `lib/swift/host/compiler/lib_Compiler*.dylib`(`lib/CompilerSwiftSyntax`): swift-syntax libraries for compiler. Library evolution is disabled. * Compiler (`ASTGen` and `swiftIDEUtilsBridging`) only depends on `lib/swift/host/compiler` libraries. * `SwiftInProcPluginServer`: In-process plugin server shared library. This has one `swift_inproc_plugins_handle_message` entry point that receives a message and return the response. * In the compiler * Add `-in-process-plugin-server-path` front-end option, which specifies the `SwiftInProcPluginServer` shared library path. * Remove `LoadedLibraryPlugin`, because all library plugins are managed by `SwiftInProcPluginServer` * Introduce abstract `CompilerPlugin` class that has 2 subclasses: * `LoadedExecutablePlugin` existing class that represents an executable plugin * `InProcessPlugins` wraps `dlopen`ed `SwiftInProcPluginServer` * Unified the code path in `TypeCheckMacros.cpp` and `ASTGen`, the difference between executable plugins and library plugins are now abstracted by `CompilerPlugin`

rintaro · 2024-06-06T18:48:31Z

apple/llvm-project#8855
apple/swift-syntax#2659
apple/swift-driver#1616
@swift-ci Please smoke test

rintaro · 2024-06-06T18:48:42Z

apple/llvm-project#8855
apple/swift-syntax#2659
apple/swift-driver#1616
@swift-ci Please build toolchain Windows

rintaro · 2024-06-06T18:55:16Z

apple/llvm-project#8855
apple/swift-syntax#2659
apple/swift-driver#1616
@swift-ci Please Test Source Compatibility

rintaro · 2024-06-06T21:28:04Z

apple/llvm-project#8855
apple/swift-syntax#2659
apple/swift-driver#1616
apple/swift-installer-scripts#304
@swift-ci Please build toolchain Windows

rintaro commented May 17, 2024

View reviewed changes

rintaro mentioned this pull request May 17, 2024

Add In-process plugin server path apple/swift-driver#1616

Open

rintaro force-pushed the macros-inproc-plugin branch 3 times, most recently from 9c69aa8 to abc1390 Compare May 21, 2024 19:59

hamishknight reviewed May 22, 2024

View reviewed changes

rintaro force-pushed the macros-inproc-plugin branch 2 times, most recently from 1d223b1 to 8e7b75e Compare May 24, 2024 04:16

rintaro commented May 24, 2024

View reviewed changes

tools/libSwiftScan/CMakeLists.txt Outdated Show resolved Hide resolved

rintaro force-pushed the macros-inproc-plugin branch from 8e7b75e to 37ae72d Compare May 24, 2024 04:59

rintaro force-pushed the macros-inproc-plugin branch from 59d6af7 to da3d080 Compare May 25, 2024 00:04

rintaro force-pushed the macros-inproc-plugin branch from da3d080 to b3c996d Compare May 25, 2024 00:51

rintaro force-pushed the macros-inproc-plugin branch from e64de00 to 9be379a Compare May 25, 2024 06:24

rintaro force-pushed the macros-inproc-plugin branch from 9be379a to 7dd4f52 Compare May 30, 2024 20:11

rintaro force-pushed the macros-inproc-plugin branch from 601d800 to 9c4a989 Compare May 31, 2024 21:00

rintaro force-pushed the macros-inproc-plugin branch from 9c4a989 to d7a0d58 Compare June 4, 2024 18:16

rintaro force-pushed the macros-inproc-plugin branch from d7a0d58 to db933a8 Compare June 5, 2024 21:51

rintaro mentioned this pull request Jun 5, 2024

[lldb] Update for _CompilerSwiftSyntax libraries apple/llvm-project#8855

Open

rintaro force-pushed the macros-inproc-plugin branch from db933a8 to bd5a47d Compare June 6, 2024 18:31

rintaro force-pushed the macros-inproc-plugin branch from bd5a47d to c0449ed Compare June 6, 2024 18:42

rintaro mentioned this pull request Jun 6, 2024

[Windows] Add _CompilerSwiftSyntax libs and plugin servers apple/swift-installer-scripts#304

Open

rintaro marked this pull request as ready for review June 6, 2024 22:53

rintaro requested review from ahoppen, bnbarham, hborla, slavapestov, xedin, CodaFi, DougGregor, artemcm and tshortli as code owners June 6, 2024 22:53

	/// Opaque value of the protocol capability of the pluugin. This is a
	/// Opaque value of the protocol capability of the plugin. This is a

	// Let ASTGen to cleanup things.
	// Let ASTGen do cleanup things.

		llvm::Expected<InProcessPlugins *>
		InProcessPlugins::create(const char *serverPath) {

	# Ensure that we can find the shaered swiftsyntax libraries.
	# Ensure that we can find the shared swiftsyntax libraries.

[Macros] In-process plugin server #73725

Are you sure you want to change the base?

[Macros] In-process plugin server #73725

Conversation

rintaro commented May 17, 2024 • edited

rintaro May 17, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rintaro May 24, 2024 • edited

Choose a reason for hiding this comment

rintaro commented May 24, 2024

rintaro commented May 24, 2024

rintaro commented May 24, 2024

rintaro commented May 25, 2024

rintaro commented May 25, 2024

rintaro commented May 25, 2024

rintaro commented May 25, 2024

rintaro commented May 25, 2024

kabiroberai commented May 29, 2024 • edited

rintaro commented May 30, 2024 • edited

rintaro commented May 30, 2024

rintaro commented May 30, 2024

kabiroberai commented May 31, 2024 • edited

rintaro commented May 31, 2024

rintaro commented May 31, 2024

rintaro commented Jun 4, 2024

rintaro commented Jun 4, 2024

rintaro commented Jun 4, 2024

rintaro commented Jun 5, 2024

rintaro commented Jun 5, 2024

rintaro commented Jun 6, 2024

rintaro commented Jun 6, 2024

rintaro commented Jun 6, 2024

rintaro commented Jun 6, 2024

rintaro commented May 17, 2024 •

edited

rintaro May 17, 2024 •

edited

rintaro May 24, 2024 •

edited

kabiroberai commented May 29, 2024 •

edited

rintaro commented May 30, 2024 •

edited

kabiroberai commented May 31, 2024 •

edited