Skip to content

Introduce try/await/unsafe macro lexical contexts with unfolded sequence handling #3037

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 4, 2025
Merged

Introduce try/await/unsafe macro lexical contexts with unfolded sequence handling #3037

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
52c4bc2
Include try and await expressions which are parents of a freestanding…
stmontgomery Jul 10, 2024
2b9a763
Handle `try` and `await` in unfolded sequences
hamishknight Apr 1, 2025
2c16aeb
Add `unsafe` as a lexical context too
hamishknight Apr 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Sources/SwiftOperators/OperatorTable+Folding.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,8 @@ extension OperatorTable {
)
)
}
// NOTE: If you add a new try/await/unsafe-like hoisting case here, make
// sure to also update `allMacroLexicalContexts` to handle it.

// The form of the binary operation depends on the operator itself,
// which will be one of the unresolved infix operators.
Expand Down
55 changes: 55 additions & 0 deletions Sources/SwiftSyntaxMacros/Syntax+LexicalContext.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,24 @@ extension SyntaxProtocol {
case let freestandingMacro as FreestandingMacroExpansionSyntax:
return Syntax(freestandingMacro.detached) as Syntax

// `try`, `await`, and `unsafe` are preserved: A freestanding expression
// macro may need to know whether those keywords are present so it can
// propagate them to any expressions in its expansion which were passed as
// arguments to the macro. The sub-expression is replaced with a trivial
// placeholder, though.
case var tryExpr as TryExprSyntax:
tryExpr = tryExpr.detached
tryExpr.expression = ExprSyntax(TypeExprSyntax(type: IdentifierTypeSyntax(name: .wildcardToken())))
return Syntax(tryExpr)
case var awaitExpr as AwaitExprSyntax:
awaitExpr = awaitExpr.detached
awaitExpr.expression = ExprSyntax(TypeExprSyntax(type: IdentifierTypeSyntax(name: .wildcardToken())))
return Syntax(awaitExpr)
case var unsafeExpr as UnsafeExprSyntax:
unsafeExpr = unsafeExpr.detached
unsafeExpr.expression = ExprSyntax(TypeExprSyntax(type: IdentifierTypeSyntax(name: .wildcardToken())))
return Syntax(unsafeExpr)

default:
return nil
}
Expand All @@ -92,6 +110,43 @@ extension SyntaxProtocol {
if let parentContext = parentNode.asMacroLexicalContext() {
parentContexts.append(parentContext)
}
// Unfolded sequence expressions require special handling - effect marker
// nodes like `try`, `await`, and `unsafe` are treated as lexical contexts
// for all the nodes on their right. Cases where they don't end up
// covering nodes to their right in the folded tree are invalid and will
// be diagnosed by the compiler. This matches the compiler's ASTScope
// handling logic.
if let sequence = parentNode.as(SequenceExprSyntax.self) {
var sequenceExprContexts: [Syntax] = []
for elt in sequence.elements {
if elt.range.contains(self.position) {
// `sequenceExprContexts` is built from the top-down, but we
// build the rest of the contexts bottom-up. Reverse for
// consistency.
parentContexts += sequenceExprContexts.reversed()
Comment on lines +123 to +126
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn’t it make more sense to move this after the for loop? Just to make sure that we add elements to parentContext in case we should terminate the loop for some other reason than the break below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are there cases where we might not be able to determine which element the node is within? The main goal here is to only add the effects for nodes that occur after them in the sequence

Copy link
Member

Choose a reason for hiding this comment

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

No, I just thought that it would read nicer to have

            // `sequenceExprContexts` is built from the top-down, but we
            // build the rest of the contexts bottom-up. Reverse for
            // consistency.
            parentContexts += sequenceExprContexts.reversed()

after the for loop and only having a break in here because the addition of elements to parentContexts conceptually happens after iterating over the elements.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah okay, I don't really feel all that strongly about it, happy to change

break
}
var elt = elt
while true {
if let tryElt = elt.as(TryExprSyntax.self) {
sequenceExprContexts.append(tryElt.asMacroLexicalContext()!)
elt = tryElt.expression
continue
}
if let awaitElt = elt.as(AwaitExprSyntax.self) {
sequenceExprContexts.append(awaitElt.asMacroLexicalContext()!)
elt = awaitElt.expression
continue
}
if let unsafeElt = elt.as(UnsafeExprSyntax.self) {
Comment on lines +131 to +141
Copy link
Member

Choose a reason for hiding this comment

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

Not for this PR, but this makes me think we need a protocol for the "effect-like" expression nodes. I feel like this pattern is going to come up a bit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Filed #3040

Copy link
Contributor

Choose a reason for hiding this comment

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

I requested that a while back, actually. :)

Copy link
Contributor

Choose a reason for hiding this comment

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

#2549

sequenceExprContexts.append(unsafeElt.asMacroLexicalContext()!)
elt = unsafeElt.expression
Copy link
Member

Choose a reason for hiding this comment

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

I would like us to include unsafe expressions in the lexical context as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to include unsafe

continue
}
break
}
}
}

currentNode = parentNode
}
Expand Down
187 changes: 184 additions & 3 deletions Tests/SwiftSyntaxMacroExpansionTest/LexicalContextTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -531,7 +531,7 @@ final class LexicalContextTests: XCTestCase {
struct S {
let arg: C
var contextDescription: String {
#lexicalContextDescription
unsafe try await #lexicalContextDescription
}
}
return S(arg: c)
Expand All @@ -542,7 +542,10 @@ final class LexicalContextTests: XCTestCase {
struct S {
let arg: C
var contextDescription: String {
"""
unsafe try await """
await _
try _
unsafe _
contextDescription: String
struct S {}
{ c in
Expand All @@ -551,7 +554,7 @@ final class LexicalContextTests: XCTestCase {
struct S {
let arg: C
var contextDescription: String {
#lexicalContextDescription
unsafe try await #lexicalContextDescription
}
}
return S(arg: c)
Expand All @@ -565,4 +568,182 @@ final class LexicalContextTests: XCTestCase {
macros: ["lexicalContextDescription": LexicalContextDescriptionMacro.self]
)
}

func testEffectMarkersInSequenceLexicalContext() {
// Valid cases.
assertMacroExpansion(
"unsafe try await #lexicalContextDescription + #lexicalContextDescription",
expandedSource: #"""
unsafe try await """
await _
try _
unsafe _
""" + """
await _
try _
unsafe _
"""
"""#,
macros: ["lexicalContextDescription": LexicalContextDescriptionMacro.self]
)
assertMacroExpansion(
"try unsafe await 0 + 1 + foo(#lexicalContextDescription) + 2",
expandedSource: #"""
try unsafe await 0 + 1 + foo("""
await _
unsafe _
try _
""") + 2
"""#,
macros: ["lexicalContextDescription": LexicalContextDescriptionMacro.self]
)
assertMacroExpansion(
"x = try await unsafe 0 + 1 + foo(#lexicalContextDescription) + 2",
expandedSource: #"""
x = try await unsafe 0 + 1 + foo("""
unsafe _
await _
try _
""") + 2
"""#,
macros: ["lexicalContextDescription": LexicalContextDescriptionMacro.self]
)
// `unsafe try await` in the 'then' branch doesn't cover condition or else.
assertMacroExpansion(
"#lexicalContextDescription ? unsafe try await #lexicalContextDescription : #lexicalContextDescription",
expandedSource: #"""
"""
""" ? unsafe try await """
await _
try _
unsafe _
""" : """
"""
"""#,
macros: ["lexicalContextDescription": LexicalContextDescriptionMacro.self]
)
// Same for else.
assertMacroExpansion(
"#lexicalContextDescription ? #lexicalContextDescription : unsafe try await #lexicalContextDescription",
expandedSource: #"""
"""
""" ? """
""" : unsafe try await """
await _
try _
unsafe _
"""
"""#,
macros: ["lexicalContextDescription": LexicalContextDescriptionMacro.self]
)
// 'unsafe try await' in the condition here covers the entire expression
assertMacroExpansion(
"unsafe try await #lexicalContextDescription ? #lexicalContextDescription : #lexicalContextDescription ~~ #lexicalContextDescription",
expandedSource: #"""
unsafe try await """
await _
try _
unsafe _
""" ? """
await _
try _
unsafe _
""" : """
await _
try _
unsafe _
""" ~~ """
await _
try _
unsafe _
"""
"""#,
macros: ["lexicalContextDescription": LexicalContextDescriptionMacro.self]
)
assertMacroExpansion(
"x = unsafe try try! await 0 + #lexicalContextDescription",
expandedSource: #"""
x = unsafe try try! await 0 + """
await _
try! _
try _
unsafe _
"""
"""#,
macros: ["lexicalContextDescription": LexicalContextDescriptionMacro.self]
)

// Invalid cases
assertMacroExpansion(
"0 + unsafe try await #lexicalContextDescription",
expandedSource: #"""
0 + unsafe try await """
await _
try _
unsafe _
"""
"""#,
macros: ["lexicalContextDescription": LexicalContextDescriptionMacro.self]
)
// The `unsafe try await` may not actually cover `lexicalContextDescription`
// here, but this will be rejected by the compiler.
assertMacroExpansion(
"0 + unsafe try await 1 ^ #lexicalContextDescription",
expandedSource: #"""
0 + unsafe try await 1 ^ """
await _
try _
unsafe _
"""
"""#,
macros: ["lexicalContextDescription": LexicalContextDescriptionMacro.self]
)
// Invalid if '^' has a lower precedence than '='.
assertMacroExpansion(
"x = unsafe try await 0 ^ #lexicalContextDescription",
expandedSource: #"""
x = unsafe try await 0 ^ """
await _
try _
unsafe _
"""
"""#,
macros: ["lexicalContextDescription": LexicalContextDescriptionMacro.self]
)
// Unassignable
assertMacroExpansion(
"#lexicalContextDescription = unsafe try await 0 + 1",
expandedSource: #"""
"""
""" = unsafe try await 0 + 1
"""#,
macros: ["lexicalContextDescription": LexicalContextDescriptionMacro.self]
)
assertMacroExpansion(
"unsafe try await #lexicalContextDescription = 0 + #lexicalContextDescription",
expandedSource: #"""
unsafe try await """
await _
try _
unsafe _
""" = 0 + """
await _
try _
unsafe _
"""
"""#,
macros: ["lexicalContextDescription": LexicalContextDescriptionMacro.self]
)
assertMacroExpansion(
"unsafe try await foo() ? 0 : 1 = #lexicalContextDescription",
expandedSource: #"""
unsafe try await foo() ? 0 : 1 = """
await _
try _
unsafe _
"""
"""#,
macros: ["lexicalContextDescription": LexicalContextDescriptionMacro.self]
)
}
}