Skip to content

Commit 28aad9d

Browse files
rintarorintaro
committed
[Macros] Return plugin loading error as result
Instead of emitting an warning to the diagnostic engine, return the plugin loading error as the result of the request. So that the user can decide to emit it as a warning or an error.
1 parent bdd4c00 commit 28aad9d

File tree

14 files changed

+188
-68
lines changed

14 files changed

+188
-68
lines changed

‎include/swift/AST/DiagnosticsFrontend.def

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -536,9 +536,6 @@ REMARK(warning_in_access_notes_file,none,
536536
"ignored invalid content in access notes file: %0",
537537
(StringRef))
538538

539-
WARNING(compiler_plugin_not_loaded,none,
540-
"compiler plugin not loaded: %0; loader error: %1", (StringRef, StringRef))
541-
542539
ERROR(dont_enable_interop_and_compat,none,
543540
"do not pass both '-enable-experimental-cxx-interop' and "
544541
"'-cxx-interoperability-mode'; remove '-enable-experimental-cxx-interop'", ())

‎include/swift/AST/MacroDefinition.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,8 @@ struct ExternalMacroDefinition {
4343
static_cast<const void *>(message.data())};
4444
}
4545
bool isError() const { return kind == PluginKind::Error; }
46-
llvm::StringRef getErrorMessage() const {
47-
return llvm::StringRef(static_cast<const char *>(opaqueHandle));
46+
NullTerminatedStringRef getErrorMessage() const {
47+
return static_cast<const char *>(opaqueHandle);
4848
}
4949
};
5050

‎include/swift/AST/PluginLoader.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,14 +76,15 @@ class PluginLoader {
7676
/// returns a nullptr.
7777
/// NOTE: This method is idempotent. If the plugin is already loaded, the same
7878
/// instance is simply returned.
79-
LoadedLibraryPlugin *loadLibraryPlugin(llvm::StringRef path);
79+
llvm::Expected<LoadedLibraryPlugin *> loadLibraryPlugin(llvm::StringRef path);
8080

8181
/// Launch the specified executable plugin path resolving the path with the
8282
/// current VFS. If it fails to load the plugin, a diagnostic is emitted, and
8383
/// returns a nullptr.
8484
/// NOTE: This method is idempotent. If the plugin is already loaded, the same
8585
/// instance is simply returned.
86-
LoadedExecutablePlugin *loadExecutablePlugin(llvm::StringRef path);
86+
llvm::Expected<LoadedExecutablePlugin *>
87+
loadExecutablePlugin(llvm::StringRef path);
8788
};
8889

8990
} // namespace swift

‎include/swift/AST/TypeCheckRequests.h

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4238,34 +4238,57 @@ class ExpandSynthesizedMemberMacroRequest
42384238
};
42394239

42404240
/// Represent a loaded plugin either an in-process library or an executable.
4241-
class LoadedCompilerPlugin {
4242-
llvm::PointerUnion<LoadedLibraryPlugin *, LoadedExecutablePlugin *> ptr;
4241+
class CompilerPluginLoadResult {
4242+
enum class PluginKind : uint8_t {
4243+
Error = 0,
4244+
Library,
4245+
Executable,
4246+
};
4247+
PluginKind Kind;
4248+
void *opaqueHandle;
4249+
4250+
CompilerPluginLoadResult(PluginKind K, void *opaque)
4251+
: Kind(K), opaqueHandle(opaque) {}
42434252

42444253
public:
4245-
LoadedCompilerPlugin(std::nullptr_t) : ptr(nullptr) {}
4246-
LoadedCompilerPlugin(LoadedLibraryPlugin *ptr) : ptr(ptr){};
4247-
LoadedCompilerPlugin(LoadedExecutablePlugin *ptr) : ptr(ptr){};
4254+
CompilerPluginLoadResult(LoadedLibraryPlugin *ptr)
4255+
: CompilerPluginLoadResult(PluginKind::Library, ptr){};
4256+
CompilerPluginLoadResult(LoadedExecutablePlugin *ptr)
4257+
: CompilerPluginLoadResult(PluginKind::Executable, ptr){};
4258+
static CompilerPluginLoadResult error(NullTerminatedStringRef message) {
4259+
return CompilerPluginLoadResult(PluginKind::Error,
4260+
const_cast<char *>(message.data()));
4261+
}
42484262

42494263
LoadedLibraryPlugin *getAsLibraryPlugin() const {
4250-
return ptr.dyn_cast<LoadedLibraryPlugin *>();
4264+
if (Kind != PluginKind::Library)
4265+
return nullptr;
4266+
return static_cast<LoadedLibraryPlugin *>(opaqueHandle);
42514267
}
42524268
LoadedExecutablePlugin *getAsExecutablePlugin() const {
4253-
return ptr.dyn_cast<LoadedExecutablePlugin *>();
4269+
if (Kind != PluginKind::Executable)
4270+
return nullptr;
4271+
return static_cast<LoadedExecutablePlugin *>(opaqueHandle);
4272+
}
4273+
bool isError() const { return Kind == PluginKind::Error; }
4274+
NullTerminatedStringRef getErrorMessage() const {
4275+
assert(isError());
4276+
return static_cast<const char *>(opaqueHandle);
42544277
}
42554278
};
42564279

42574280
class CompilerPluginLoadRequest
42584281
: public SimpleRequest<CompilerPluginLoadRequest,
4259-
LoadedCompilerPlugin(ASTContext *, Identifier),
4282+
CompilerPluginLoadResult(ASTContext *, Identifier),
42604283
RequestFlags::Cached> {
42614284
public:
42624285
using SimpleRequest::SimpleRequest;
42634286

42644287
private:
42654288
friend SimpleRequest;
42664289

4267-
LoadedCompilerPlugin evaluate(Evaluator &evaluator, ASTContext *ctx,
4268-
Identifier moduleName) const;
4290+
CompilerPluginLoadResult evaluate(Evaluator &evaluator, ASTContext *ctx,
4291+
Identifier moduleName) const;
42694292

42704293
public:
42714294
// Source location

‎include/swift/AST/TypeCheckerTypeIDZone.def

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -458,7 +458,7 @@ SWIFT_REQUEST(TypeChecker, MacroDefinitionRequest,
458458
MacroDefinition(MacroDecl *),
459459
Cached, NoLocationInfo)
460460
SWIFT_REQUEST(TypeChecker, CompilerPluginLoadRequest,
461-
LoadedCompilerPlugin(ASTContext *, Identifier),
461+
CompilerPluginLoadResult(ASTContext *, Identifier),
462462
Cached, NoLocationInfo)
463463
SWIFT_REQUEST(TypeChecker, ExternalMacroDefinitionRequest,
464464
ExternalMacroDefinition(ASTContext *, Identifier, Identifier),

‎include/swift/Basic/StringExtras.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -497,6 +497,10 @@ class NullTerminatedStringRef {
497497
NullTerminatedStringRef(llvm::Twine Str, Allocator &A) : Ref("") {
498498
if (Str.isTriviallyEmpty())
499499
return;
500+
if (Str.isSingleStringLiteral()) {
501+
Ref = Str.getSingleStringRef();
502+
return;
503+
}
500504
llvm::SmallString<0> stash;
501505
auto _ref = Str.toStringRef(stash);
502506

‎lib/AST/PluginLoader.cpp

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -158,13 +158,12 @@ PluginLoader::lookupPluginByModuleName(Identifier moduleName) {
158158
}
159159
}
160160

161-
LoadedLibraryPlugin *PluginLoader::loadLibraryPlugin(StringRef path) {
161+
llvm::Expected<LoadedLibraryPlugin *>
162+
PluginLoader::loadLibraryPlugin(StringRef path) {
162163
auto fs = Ctx.SourceMgr.getFileSystem();
163164
SmallString<128> resolvedPath;
164165
if (auto err = fs->getRealPath(path, resolvedPath)) {
165-
Ctx.Diags.diagnose(SourceLoc(), diag::compiler_plugin_not_loaded, path,
166-
err.message());
167-
return nullptr;
166+
return llvm::createStringError(err, err.message());
168167
}
169168

170169
// Track the dependency.
@@ -174,21 +173,25 @@ LoadedLibraryPlugin *PluginLoader::loadLibraryPlugin(StringRef path) {
174173
// Load the plugin.
175174
auto plugin = getRegistry()->loadLibraryPlugin(resolvedPath);
176175
if (!plugin) {
177-
Ctx.Diags.diagnose(SourceLoc(), diag::compiler_plugin_not_loaded, path,
178-
llvm::toString(plugin.takeError()));
179-
return nullptr;
176+
resolvedPath.push_back(0);
177+
return llvm::handleErrors(
178+
plugin.takeError(), [&](const llvm::ErrorInfoBase &err) {
179+
return llvm::createStringError(
180+
err.convertToErrorCode(),
181+
"compiler plugin not loaded: %s; loader error: %s",
182+
resolvedPath.data(), err.message().data());
183+
});
180184
}
181185

182-
return plugin.get();
186+
return plugin;
183187
}
184188

185-
LoadedExecutablePlugin *PluginLoader::loadExecutablePlugin(StringRef path) {
189+
llvm::Expected<LoadedExecutablePlugin *>
190+
PluginLoader::loadExecutablePlugin(StringRef path) {
186191
auto fs = Ctx.SourceMgr.getFileSystem();
187192
SmallString<128> resolvedPath;
188193
if (auto err = fs->getRealPath(path, resolvedPath)) {
189-
Ctx.Diags.diagnose(SourceLoc(), diag::compiler_plugin_not_loaded, path,
190-
err.message());
191-
return nullptr;
194+
return llvm::createStringError(err, err.message());
192195
}
193196

194197
// Track the dependency.
@@ -198,10 +201,15 @@ LoadedExecutablePlugin *PluginLoader::loadExecutablePlugin(StringRef path) {
198201
// Load the plugin.
199202
auto plugin = getRegistry()->loadExecutablePlugin(resolvedPath);
200203
if (!plugin) {
201-
Ctx.Diags.diagnose(SourceLoc(), diag::compiler_plugin_not_loaded, path,
202-
llvm::toString(plugin.takeError()));
203-
return nullptr;
204+
resolvedPath.push_back(0);
205+
return llvm::handleErrors(
206+
plugin.takeError(), [&](const llvm::ErrorInfoBase &err) {
207+
return llvm::createStringError(
208+
err.convertToErrorCode(),
209+
"compiler plugin not loaded: %s; loader error: %s",
210+
resolvedPath.data(), err.message().data());
211+
});
204212
}
205213

206-
return plugin.get();
214+
return plugin;
207215
}

‎lib/ASTGen/Sources/ASTGen/PluginHost.swift

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,8 @@ public func _initializePlugin(
3737
try plugin.initialize()
3838
return true
3939
} catch {
40-
diagEngine?.diagnose(
41-
message: "compiler plugin not loaded: '\(plugin.executableFilePath); failed to initialize",
42-
severity: .warning)
40+
// Don't care the actual error. Probably the plugin is completely broken.
41+
// The failure is diagnosed in the caller.
4342
return false
4443
}
4544
}

‎lib/Sema/TypeCheckMacros.cpp

Lines changed: 48 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ MacroDefinition MacroDefinitionRequest::evaluate(
277277
#endif
278278
}
279279

280-
static LoadedExecutablePlugin *
280+
static llvm::Expected<LoadedExecutablePlugin *>
281281
initializeExecutablePlugin(ASTContext &ctx,
282282
LoadedExecutablePlugin *executablePlugin,
283283
StringRef libraryPath, Identifier moduleName) {
@@ -291,7 +291,10 @@ initializeExecutablePlugin(ASTContext &ctx,
291291
if (!executablePlugin->isInitialized()) {
292292
#if SWIFT_BUILD_SWIFT_SYNTAX
293293
if (!swift_ASTGen_initializePlugin(executablePlugin, &ctx.Diags)) {
294-
return nullptr;
294+
return llvm::createStringError(
295+
llvm::inconvertibleErrorCode(),
296+
"failed to initialize executable plugin '" +
297+
StringRef(executablePlugin->getExecutablePath()) + "'");
295298
}
296299

297300
// Resend the compiler capability on reconnect.
@@ -314,9 +317,7 @@ initializeExecutablePlugin(ASTContext &ctx,
314317
llvm::SmallString<128> resolvedLibraryPath;
315318
auto fs = ctx.SourceMgr.getFileSystem();
316319
if (auto err = fs->getRealPath(libraryPath, resolvedLibraryPath)) {
317-
ctx.Diags.diagnose(SourceLoc(), diag::compiler_plugin_not_loaded,
318-
executablePlugin->getExecutablePath(), err.message());
319-
return nullptr;
320+
return llvm::createStringError(err, err.message());
320321
}
321322
std::string resolvedLibraryPathStr(resolvedLibraryPath);
322323
std::string moduleNameStr(moduleName.str());
@@ -325,7 +326,11 @@ initializeExecutablePlugin(ASTContext &ctx,
325326
executablePlugin, resolvedLibraryPathStr.c_str(), moduleNameStr.c_str(),
326327
&ctx.Diags);
327328
if (!loaded)
328-
return nullptr;
329+
return llvm::createStringError(
330+
llvm::inconvertibleErrorCode(),
331+
"failed to load library plugin '" + resolvedLibraryPathStr +
332+
"' in plugin server '" +
333+
StringRef(executablePlugin->getExecutablePath()) + "'");
329334

330335
// Set a callback to load the library again on reconnections.
331336
auto *callback = new std::function<void(void)>(
@@ -348,37 +353,59 @@ initializeExecutablePlugin(ASTContext &ctx,
348353
return executablePlugin;
349354
}
350355

351-
LoadedCompilerPlugin
356+
CompilerPluginLoadResult
352357
CompilerPluginLoadRequest::evaluate(Evaluator &evaluator, ASTContext *ctx,
353358
Identifier moduleName) const {
354359
PluginLoader &loader = ctx->getPluginLoader();
355360
const auto &entry = loader.lookupPluginByModuleName(moduleName);
356361

362+
std::string errorMessage;
363+
357364
if (!entry.executablePath.empty()) {
358-
if (LoadedExecutablePlugin *executablePlugin =
359-
loader.loadExecutablePlugin(entry.executablePath)) {
365+
llvm::Expected<LoadedExecutablePlugin *> executablePlugin =
366+
loader.loadExecutablePlugin(entry.executablePath);
367+
if (executablePlugin) {
360368
if (ctx->LangOpts.EnableMacroLoadingRemarks) {
361369
unsigned tag = entry.libraryPath.empty() ? 1 : 2;
362370
ctx->Diags.diagnose(SourceLoc(), diag::macro_loaded, moduleName, tag,
363371
entry.executablePath, entry.libraryPath);
364372
}
365373

366-
return initializeExecutablePlugin(*ctx, executablePlugin,
367-
entry.libraryPath, moduleName);
374+
executablePlugin = initializeExecutablePlugin(
375+
*ctx, executablePlugin.get(), entry.libraryPath, moduleName);
368376
}
377+
if (executablePlugin)
378+
return executablePlugin.get();
379+
llvm::handleAllErrors(
380+
executablePlugin.takeError(),
381+
[&](const llvm::ErrorInfoBase &err) { errorMessage += err.message(); });
369382
} else if (!entry.libraryPath.empty()) {
370-
if (LoadedLibraryPlugin *libraryPlugin =
371-
loader.loadLibraryPlugin(entry.libraryPath)) {
383+
384+
llvm::Expected<LoadedLibraryPlugin *> libraryPlugin =
385+
loader.loadLibraryPlugin(entry.libraryPath);
386+
if (libraryPlugin) {
372387
if (ctx->LangOpts.EnableMacroLoadingRemarks) {
373388
ctx->Diags.diagnose(SourceLoc(), diag::macro_loaded, moduleName, 0,
374389
entry.libraryPath, StringRef());
375390
}
376391

377-
return libraryPlugin;
392+
return libraryPlugin.get();
393+
} else {
394+
llvm::handleAllErrors(libraryPlugin.takeError(),
395+
[&](const llvm::ErrorInfoBase &err) {
396+
errorMessage += err.message();
397+
});
378398
}
379399
}
380-
381-
return nullptr;
400+
if (!errorMessage.empty()) {
401+
NullTerminatedStringRef err(errorMessage, *ctx);
402+
return CompilerPluginLoadResult::error(err);
403+
} else {
404+
NullTerminatedStringRef errMsg("plugin that can handle module '" +
405+
moduleName.str() + "' not found",
406+
*ctx);
407+
return CompilerPluginLoadResult::error(errMsg);
408+
}
382409
}
383410

384411
static ExternalMacroDefinition
@@ -431,6 +458,8 @@ resolveExecutableMacro(ASTContext &ctx,
431458
return ExternalMacroDefinition{
432459
ExternalMacroDefinition::PluginKind::Executable, execMacro};
433460
}
461+
// NOTE: this is not reachable because executable macro resolution always
462+
// succeeds.
434463
NullTerminatedStringRef err(
435464
"macro implementation type '" + moduleName.str() + "." + typeName.str() +
436465
"' could not be found in executable plugin" +
@@ -448,8 +477,8 @@ ExternalMacroDefinitionRequest::evaluate(Evaluator &evaluator, ASTContext *ctx,
448477
// Try to load a plugin module from the plugin search paths. If it
449478
// succeeds, resolve in-process from that plugin
450479
CompilerPluginLoadRequest loadRequest{ctx, moduleName};
451-
LoadedCompilerPlugin loaded =
452-
evaluateOrDefault(evaluator, loadRequest, nullptr);
480+
CompilerPluginLoadResult loaded = evaluateOrDefault(
481+
evaluator, loadRequest, CompilerPluginLoadResult::error("request error"));
453482

454483
if (auto loadedLibrary = loaded.getAsLibraryPlugin()) {
455484
return resolveInProcessMacro(*ctx, moduleName, typeName, loadedLibrary);
@@ -459,10 +488,7 @@ ExternalMacroDefinitionRequest::evaluate(Evaluator &evaluator, ASTContext *ctx,
459488
return resolveExecutableMacro(*ctx, executablePlugin, moduleName, typeName);
460489
}
461490

462-
NullTerminatedStringRef err("plugin that can handle module '" +
463-
moduleName.str() + "' not found",
464-
*ctx);
465-
return ExternalMacroDefinition::error(err);
491+
return ExternalMacroDefinition::error(loaded.getErrorMessage());
466492
}
467493

468494
/// Adjust the given mangled name for a macro expansion to produce a valid

‎test/Macros/macro_plugin_broken.swift

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,8 @@
1818

1919
// RUN: c-index-test -read-diagnostics %t/macro_expand.dia 2>&1 | %FileCheck -check-prefix CHECK %s
2020

21-
// CHECK: (null):0:0: warning: compiler plugin not loaded: {{.+}}broken-plugin; failed to initialize
22-
// CHECK: test.swift:1:33: warning: external macro implementation type 'TestPlugin.FooMacro' could not be found for macro 'fooMacro'
23-
// CHECK: test.swift:4:7: error: external macro implementation type 'TestPlugin.FooMacro' could not be found for macro 'fooMacro'
21+
// CHECK: test.swift:1:33: warning: external macro implementation type 'TestPlugin.FooMacro' could not be found for macro 'fooMacro'; failed to initialize executable plugin '{{.*}}broken-plugin'
22+
// CHECK: test.swift:4:7: error: external macro implementation type 'TestPlugin.FooMacro' could not be found for macro 'fooMacro'; failed to initialize executable plugin '{{.*}}broken-plugin'
2423
// CHECK: +-{{.+}}test.swift:1:33: note: 'fooMacro' declared here
2524

2625
//--- test.swift

0 commit comments

Comments
 (0)