[Macro] Handle error during initializing executable plugins #65630
Conversation
rintaro
requested review from
hborla,
slavapestov,
xedin,
zoecarver and
CodaFi
as code owners
|
@swift-ci Please smoke test |
| try plugin.initialize() | ||
| return true | ||
| } catch { | ||
| // Diagnostics are emitted in the caller. |
There was a problem hiding this comment.
If we're passing in cxxDiagnosticEngine now, couldn't we just emit it here rather than return bool?
There was a problem hiding this comment.
We can, but there's no context information such as macro name, plugin path, etc. So the diagnostics here would be less useful.
There was a problem hiding this comment.
Could pass that down 🤷. Seems like at some point we either need to pass down a string that we set here (for "what happened"), or pass down the path that we then use here to emit (or some description of the plugin).
Is passing the diagnostic engine left over?
There was a problem hiding this comment.
It was intentional leftover. I can remove it if you prefer to do so.
Or, if we really want to diagnose it here, instead of passing it down, we probably should make a CompilerPlugin API to get it. But the information we have here is not really useful as well it's just "failed to receive the response".
There was a problem hiding this comment.
Up to you, it doesn't seem super useful to me if we're not using it (we can always add it back later).
There was a problem hiding this comment.
I went ahead and fixed the FIXME in this PR. Added CompilerPlugin.executableFilePath property.
Previously, the compiler fails to read the result from 'getCapability' IPC message, it used to just emit a fatalError and crashed. Instead, propagate the error status and diagnose it. rdar://108022847
rintaro
force-pushed
the
macro-plugin-intiialize-rdar108022847
branch
from
aaa8b4a to
e55a7e2
Compare
|
@swift-ci Please smoke test |
| case failedToSendMessage = "couldn't send request to plugin" | ||
| case failedToReceiveMessage = "couldn't receive result from plugin" |
There was a problem hiding this comment.
failed instead of couldn't sounds a little better to me, but otherwise looks good!
rintaro
force-pushed
the
macro-plugin-intiialize-rdar108022847
branch
from
e55a7e2 to
e181a4d
Compare
|
@swift-ci Please smoke test |
|
@swift-ci Please smoke test macOS |
Previously, when the compiler fails to read the result from 'getCapability' IPC message, it used to just emit a fatalError and crashed. Instead, propagate the error status and diagnose it.
rdar://108022847