Allow peer macros on variables with multiple bindings in assertMacroExpansion#3342
Open
adityasingh2400 wants to merge 1 commit into
Open
Allow peer macros on variables with multiple bindings in assertMacroExpansion#3342adityasingh2400 wants to merge 1 commit into
adityasingh2400 wants to merge 1 commit into
Conversation
…xpansion assertMacroExpansion rejected peer macros attached to a variable declaration with more than one binding (such as `var a: Int, b: Int`) with the error "peer macro can only be applied to a single variable", before the macro was ever invoked. This restriction was added back when the compiler effectively disallowed this by invoking the macro once per binding with the same syntax node, which produced duplicate peers and a redeclaration error. That is no longer the case: the compiler invokes a peer macro on such declarations and only diagnoses a problem if the macro itself emits conflicting declarations. A peer macro that returns no peers, or peers with unique names, compiles cleanly. This was verified by building a peer macro against the Swift 6.3 toolchain. As a result, assertMacroExpansion produced a false-positive diagnostic for code that the compiler accepts, making it an unreliable test oracle for peer macros on multi-binding variables. Remove the binding-count guard from both expandPeerMacroMember and expandPeerMacroCodeItem so the peer macro runs and its peers are emitted, matching the compiler. The corresponding error case is dropped since it is no longer used. The accessor-macro restriction is left in place because the compiler genuinely does not allow accessor macros on multi-binding variables.
adityasingh2400
requested review from
ahoppen,
hamishknight,
hborla and
rintaro
as code owners
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
assertMacroExpansionrejects a peer macro attached to a variable declaration with more than one binding, such asvar a: Int, b: Int, raising "peer macro can only be applied to a single variable" before the macro is ever invoked. Reported in #3320.This restriction dates back to #2154, when the rationale was that the compiler effectively disallowed this: it invoked the peer macro once per binding with the same syntax node, which generated the same peers twice and produced a redeclaration error. That is no longer the situation. The compiler now invokes a peer macro on such declarations and only diagnoses a problem if the macro itself emits conflicting declarations. A peer macro that returns no peers (the case in #3320), or that returns peers with unique names, compiles cleanly.
I verified this against the Swift 6.3 toolchain by building a peer macro and applying it to
var a: Int = 1, b: Int = 2:[]compiles and runs with no diagnostics.context.makeUniqueNamebased name compiles cleanly.So the guard made
assertMacroExpansionemit a false-positive diagnostic for code the compiler accepts, which makes it an unreliable oracle when testing peer macros on multi-binding variables.The fix removes the binding-count guard from both
expandPeerMacroMemberandexpandPeerMacroCodeItem, so the peer macro runs on the whole declaration and its peers are emitted, matching the compiler. The now-unusedpeerMacroOnVariableWithMultipleBindingserror case is dropped. The accessor-macro restriction is intentionally left untouched, since the compiler genuinely does not allow accessor macros on multi-binding variables.The existing
testPeerMacroOnVariableWithMultipleBindingsis updated to assert that the peer is now emitted, and a newtestPeerMacroReturningNoPeersOnVariableWithMultipleBindingscovers the exact scenario from #3320. Both fail against the current code with the spurious diagnostic and pass after the change. The full SwiftSyntaxMacroExpansion test target (135 tests) passes.