[Macros] Add executable plugin support #63793

rintaro · 2023-02-21T00:02:22Z

Executable compiler plugins are programs invoked by the host compiler and communicate with the host with IPC via standard IO (stdin/stdout.) Each message is serialized in JSON, prefixed with a header which is a 64bit little-endian integer indicating the size of the message.

Basic/ExecuteWithPipe: External program invocation. Like llvm::sys::ExecuteNoWait() but establishing pipes to the child's stdin/stdout
Basic/Sandbox: Sandboxed execution helper. Mutates command line arguments to be executed in sandbox environment (similar to SwiftPM's plugin sandbox)
AST/PluginRepository: ASTContext independent plugin manager.
ASTGen/PluginHost: Communication with the plugin. Messages are serialized by ASTGen/LLVMJSON added in [ASTGen] Add Encoder/Decoder implementation backed by llvm::json::Value #63689

This PR implements 3 messages (ASTGen/PluginMessages).

getCapability: get plugin capability. currently it only returns fixed protocolVersion 1.
expandFreestandingMacro: expand freestanding macros
expandAttachedMacro: expand attached macros

rdar://101508815

rintaro · 2023-02-21T00:02:31Z

@swift-ci Please smoke test

rintaro · 2023-02-21T03:51:43Z

@swift-ci Please smoke test

rintaro · 2023-02-21T05:10:35Z

@swift-ci Please smoke test

rintaro · 2023-02-21T06:29:38Z

@swift-ci Please smoke test

rintaro · 2023-02-21T23:53:20Z

@swift-ci Please smoke test

rintaro · 2023-02-22T17:04:02Z

@swift-ci Please Python lint

rintaro · 2023-02-22T18:04:57Z

@swift-ci Please test

Executable compiler plugins are programs invoked by the host compiler and communicate with the host with IPC via standard IO (stdin/stdout.) Each message is serialized in JSON, prefixed with a header which is a 64bit little-endian integer indicating the size of the message. * Basic/ExecuteWithPipe: External program invocation. Lik llvm::sys::ExecuteNoWait() but establishing pipes to the child's stdin/stdout * Basic/Sandbox: Sandboxed execution helper. Create command line arguments to be executed in sandbox environment (similar to SwiftPM's pluging sandbox) * AST/PluginRepository: ASTContext independent plugin manager * ASTGen/PluginHost: Communication with the plugin. Messages are serialized by ASTGen/LLVMJSON rdar://101508815

rintaro · 2023-02-22T18:22:32Z

@swift-ci Please test

bnbarham

Very cool!

bnbarham · 2023-02-22T22:02:20Z

include/swift/AST/CASTBridging.h

+/// pointer, it's not used in AST at all.
+void Plugin_setCapability(void *handle, const void *data);
+
+/// Get a coapability data set by \c Plugin_setCapability .


Suggested change

/// Get a coapability data set by \c Plugin_setCapability .

/// Get a capability data set by \c Plugin_setCapability .

bnbarham · 2023-02-22T22:05:19Z

include/swift/AST/CASTBridging.h

+
+/// Set a capability data to the plugin object. Since the data is just a opaque
+/// pointer, it's not used in AST at all.
+void Plugin_setCapability(void *handle, const void *data);


Thoughts on adding typedefs for the various void*? I know they're all still void* regardless, it just makes it easier when reading through the API (IMO).

I agree. done.

bnbarham · 2023-02-22T22:16:47Z

lib/Basic/Program.cpp

+
+    // Execute the program.
+    if (env.has_value()) {
+      const char **envp = toNullTerminatedCStringArray(*env, Alloc);


We already have envp above, assuming I'm reading the #if's correctly.

Thanks. I added "has posix_spawn" branch after fork implementation. This was a leftover.

bnbarham · 2023-02-22T22:17:40Z

include/swift/Basic/Program.h

+/// \param env An optional array of strings to use for the program's
+/// environment.


Maybe worth saying it's an array of "key=value" strings?

bnbarham · 2023-02-22T22:25:31Z

lib/AST/ASTContext.cpp


  /// Cache of loaded symbols.
  llvm::StringMap<void *> LoadedSymbols;

+  /// Map a module name to an executable plugin path that provides the module.
+  llvm::DenseMap<Identifier, std::string> ExecutablePluginPaths;


We could shove the paths into the ASTContext's allocator instead since we never remove them. Doesn't really matter, but would mean our size calculations are a little more accurate. (see AllocateCopy)

bnbarham · 2023-02-22T23:06:56Z

lib/Sema/TypeCheckMacros.cpp

+}
+
+static Optional<ExternalMacroDefinition>
+resolvExecutableMacro(ASTContext &ctx, Identifier moduleName,


Suggested change

resolvExecutableMacro(ASTContext &ctx, Identifier moduleName,

resolveExecutableMacro(ASTContext &ctx, Identifier moduleName,

bnbarham · 2023-02-22T23:09:07Z

lib/Sema/TypeCheckMacros.cpp

+    });
+  }
+
+  if (auto execMacro = swift_ASTGen_resolveExecutableMacro(


It's nice to keep the * with auto to make it obvious IMO, up to you though.

bnbarham · 2023-02-22T23:14:01Z

utils/mock_plugin.py

+# respond. For example:
+#   {'expected': {'testRequest': {'params': []}},
+#    'response': {'testRequestResult': [1,2,"foo"]}}
+# this spec matches a request that is an object with 'testRequst' key with an


Suggested change

# this spec matches a request that is an object with 'testRequst' key with an

# this spec matches a request that is an object with 'testRequest' key with an

bnbarham · 2023-02-22T23:17:18Z

lib/ASTGen/Sources/ASTGen/Macros.swift

+  moduleNameLength: Int,
+  typeName: UnsafePointer<UInt8>,
+  typeNameLength: Int,
+  pluginOpaqueHanle: UnsafeMutableRawPointer


Suggested change

pluginOpaqueHanle: UnsafeMutableRawPointer

pluginOpaqueHandle: UnsafeMutableRawPointer

bnbarham · 2023-02-22T23:26:37Z

lib/ASTGen/Sources/ASTGen/Macros.swift

+  let discriminator = String(decoding: discriminatorBuffer, as: UTF8.self)
+
+  let expandedSources: [String]?
+  switch MacroPluginKind(rawValue: macroKind)! {


Possibly over engineering here, but we could make a protocol with the two expand functions instead. They could be static or actual methods, where MacroPluginKind has an expander that returns the type or a new struct (depending on whether you choose static methods or not):

enum MacroPluginKind { ... var expander: some MacroExpander { switch ... }

That's a good idea. Let me do it in a follow-ups because it changes the indentation of the current code, which makes the change set large.

lib/ASTGen/Sources/ASTGen/Macros.swift

rintaro · 2023-02-23T01:59:15Z

@swift-ci Please test

lib/AST/PluginRegistry.cpp

rintaro · 2023-02-23T06:41:06Z

@swift-ci Please test

bnbarham · 2023-02-23T18:19:05Z

lib/AST/PluginRegistry.cpp

  }
 #else
  lib = dlopen(path.str().c_str(), RTLD_LAZY | RTLD_LOCAL);
  if (!lib) {
-    return llvm::createStringError(std::errc::not_supported,
-                                   "unsupported platform");
+    return llvm::createStringError(std::error_code(), dlerror());


If there's no suitable errc here it's probably better to use inconvertibleErrorCode

bnbarham · 2023-02-23T18:20:45Z

lib/AST/CASTBridging.cpp

@@ -651,14 +651,19 @@ void Plugin_unlock(PluginHandle handle) {
 bool Plugin_sendMessage(PluginHandle handle, const BridgedData data) {
  auto *plugin = static_cast<LoadedExecutablePlugin *>(handle);
  StringRef message(data.baseAddress, data.size);
-  return plugin->sendMessage(message);
+  return bool(plugin->sendMessage(message));


I'm fairly sure this will still fail when there's an error. LLVM has a consumeError to just eat it, though that makes me wonder if we should be passing in an error string to set.

rintaro · 2023-02-23T18:33:48Z

@swift-ci Please smoke test

rintaro force-pushed the macro-plugin-executable branch 2 times, most recently from e5e8e3e to c90931e Compare February 21, 2023 03:51

rintaro force-pushed the macro-plugin-executable branch from c90931e to 97bfd88 Compare February 21, 2023 05:10

rintaro force-pushed the macro-plugin-executable branch from 97bfd88 to b940a47 Compare February 21, 2023 06:25

rintaro force-pushed the macro-plugin-executable branch 3 times, most recently from 9040212 to 6fefff5 Compare February 21, 2023 23:53

rintaro requested a review from bnbarham February 22, 2023 00:58

rintaro force-pushed the macro-plugin-executable branch from 6fefff5 to 43f3364 Compare February 22, 2023 17:03

rintaro force-pushed the macro-plugin-executable branch from 43f3364 to dd256f2 Compare February 22, 2023 18:04

rintaro changed the title ~~[WIP][Macros] Add executable plugin support~~ [Macros] Add executable plugin support Feb 22, 2023

rintaro marked this pull request as ready for review February 22, 2023 18:05

rintaro requested review from hborla, slavapestov, xedin, zoecarver and CodaFi as code owners February 22, 2023 18:05

rintaro force-pushed the macro-plugin-executable branch from dd256f2 to 0e31393 Compare February 22, 2023 18:22

xedin removed their request for review February 22, 2023 18:23

bnbarham reviewed Feb 22, 2023

View reviewed changes

[ASTGen] Remove unnecessary parameters from the expansion funcs

e840409

rintaro commented Feb 22, 2023

View reviewed changes

lib/ASTGen/Sources/ASTGen/Macros.swift Outdated Show resolved Hide resolved

rintaro added 3 commits February 22, 2023 17:11

[CASTBridging] Typedefs for opaque pointer types

396c227

[AST] PluginRegistry returns Error/Expected instead of errorMsg out

c5a737f

[Macros] Small changes for review

c6387a5

rintaro commented Feb 23, 2023

View reviewed changes

lib/AST/PluginRegistry.cpp Outdated Show resolved Hide resolved

[AST] Fix Windows compile error

623043e

rintaro mentioned this pull request Feb 23, 2023

[Macros] Add test for macro compiler plugins swiftlang/swift-integration-tests#111

Closed

bnbarham approved these changes Feb 23, 2023

View reviewed changes

[PluginRegistry] Error handling tweaks

138934e

rintaro mentioned this pull request Feb 23, 2023

[Macros] Add executable compiler plugin support library swiftlang/swift-syntax#1359

Merged

rintaro merged commit 4190cb7 into swiftlang:main Feb 23, 2023

rintaro deleted the macro-plugin-executable branch February 23, 2023 22:02

	/// Get a coapability data set by \c Plugin_setCapability .
	/// Get a capability data set by \c Plugin_setCapability .

		/// \param env An optional array of strings to use for the program's
		/// environment.

	resolvExecutableMacro(ASTContext &ctx, Identifier moduleName,
	resolveExecutableMacro(ASTContext &ctx, Identifier moduleName,

	# this spec matches a request that is an object with 'testRequst' key with an
	# this spec matches a request that is an object with 'testRequest' key with an

	pluginOpaqueHanle: UnsafeMutableRawPointer
	pluginOpaqueHandle: UnsafeMutableRawPointer

[Macros] Add executable plugin support #63793

[Macros] Add executable plugin support #63793

Uh oh!

Conversation

rintaro commented Feb 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rintaro commented Feb 21, 2023

Uh oh!

rintaro commented Feb 21, 2023

Uh oh!

rintaro commented Feb 21, 2023

Uh oh!

rintaro commented Feb 21, 2023

Uh oh!

rintaro commented Feb 21, 2023

Uh oh!

rintaro commented Feb 22, 2023

Uh oh!

rintaro commented Feb 22, 2023

Uh oh!

rintaro commented Feb 22, 2023

Uh oh!

bnbarham left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rintaro commented Feb 23, 2023

Uh oh!

Uh oh!

rintaro commented Feb 23, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rintaro commented Feb 23, 2023

Uh oh!

Uh oh!

rintaro commented Feb 21, 2023 •

edited

Loading