Skip to content

Conversation

@DougGregor
Copy link
Member

Macros can only have a single freestanding macro role, so that their
expansion is unambiguous. However, that restriction is at the level of a
macro declaration, and is not reflected in a similar restriction on the
set of macro protocols to which a macro implementation can conform.
Therefore, have the compiler pass a freestanding macro role through to
macro expansion in the same way that we do for attached macros, and
use that to determine which macro protocol to use.

Part of fixing rdar://110418969.

Macros can only have a single freestanding macro role, so that their
expansion is unambiguous. However, that restriction is at the level of a
macro declaration, and is not reflected in a similar restriction on the
set of macro protocols to which a macro implementation can conform.
Therefore, have the compiler pass a freestanding macro role through to
macro expansion in the same way that we do for attached macros, and
use that to determine which macro protocol to use.

Part of fixing rdar://110418969.
@DougGregor DougGregor requested a review from ahoppen as a code owner June 9, 2023 20:39
@DougGregor
Copy link
Member Author

@swift-ci please test

Comment on lines +52 to +59
switch pluginMacroRole {
case .expression: macroRole = .expression
case .declaration: macroRole = .declaration
case .codeItem: macroRole = .codeItem

case .accessor, .conformance, .member, .memberAttribute, .peer:
throw MacroExpansionError.invalidMacroRole(pluginMacroRole)
}
Copy link
Member

@rintaro rintaro Jun 9, 2023

Choose a reason for hiding this comment

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

(Not critical at all)
Could you use MacroRole.init(messageMacroRole: PluginMessage.MacroRole) (at the bottom of this file) here?
Since expandFreestandingMacro diagnoses unmatched macro role, and invalid macro role here would be a compiler error, diagnosing invalid is not necessary I think.

Copy link
Member

Choose a reason for hiding this comment

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

But if we do diagnose invalid role here, we should do it in expandAttachedMacro() 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.

I'd rather be defensive here, because we're on one side of an IPC barrier

Copy link
Member Author

Choose a reason for hiding this comment

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

Nevermind, I see what you mean. It'll be checked and produce an error down in expandFreestandingMacro, so we don't need the redundant check here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Cleanup is at #1762

@DougGregor DougGregor merged commit 4d8981f into swiftlang:main Jun 10, 2023
@DougGregor DougGregor deleted the freestanding-macro-role-message branch June 10, 2023 00:04
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