Skip to content

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Aug 29, 2023

If the macro expansion of a freestanding expression macro throws an error, expandCodeBlockItem returned nil while adding the thrown error to the macro expansion context. visit(_:CodeBlockItemListSyntax).addResult took the nil return value as an indicator that the macro wasn’t expanded because its macro definition wasn’t found and ended up calling the expansion again in

// Recurse on the child node
newItems.append(visit(node))

Change the return value of expandCodeBlockItem to an enum that indicates whether the macro was not found or if the expansion failed. If the macro was found but the expansion threw an error, we just just retain the macro as-is without calling into visit again.

Fixes #2111
rdar://114592410

@ahoppen
Copy link
Member Author

ahoppen commented Aug 30, 2023

@swift-ci Please test

@ahoppen
Copy link
Member Author

ahoppen commented Aug 31, 2023

@swift-ci Please test Linux

1 similar comment
@ahoppen
Copy link
Member Author

ahoppen commented Aug 31, 2023

@swift-ci Please test Linux

case failure

/// The node that should be expanded was not a macro known to the macro system.
case notAMacro
Copy link
Contributor

Choose a reason for hiding this comment

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

unknownMacro

…wice in MacroSystem

If the macro expansion of a freestanding expression macro throws an error, `expandCodeBlockItem` returned `nil` while adding the thrown error to the macro expansion context. `visit(_:CodeBlockItemListSyntax).addResult` took the `nil` return value as an indicator that the macro wasn’t expanded because its macro definition wasn’t found and ended up calling the expansion again in

```swift
// Recurse on the child node
newItems.append(visit(node))
```

Change the return value of `expandCodeBlockItem` to an enum that indicates whether the macro was not found or if the expansion failed. If the macro was found but the expansion threw an error, we just just retain the macro as-is without calling into `visit` again.

Fixes swiftlang#2111
rdar://114592410
@ahoppen ahoppen force-pushed the ahoppen/multiple-errors branch from 2c48690 to c32dca9 Compare September 1, 2023 16:17
@ahoppen
Copy link
Member Author

ahoppen commented Sep 1, 2023

@swift-ci Please test

@ahoppen
Copy link
Member Author

ahoppen commented Sep 1, 2023

@swift-ci Please test Windows

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.

assertMacroExpansion reports duplicate diagnostic
2 participants