Skip to content

Conversation

bnbarham
Copy link
Contributor

@bnbarham bnbarham commented Jun 9, 2023

Invalid macro expansions should generate an assertion in assertMacroExpansion rather than being ignored.

@bnbarham bnbarham requested a review from rintaro June 9, 2023 22:17
@bnbarham bnbarham requested a review from ahoppen as a code owner June 9, 2023 22:17
@bnbarham
Copy link
Contributor Author

bnbarham commented Jun 9, 2023

@swift-ci please test

@bnbarham bnbarham requested a review from DougGregor June 9, 2023 22:19
let expandedSourceFile = origSourceFile.expand(macros: macros, in: context)
let formattedSourceFile = expandedSourceFile.formatted(using: BasicFormat(indentationWidth: indentationWidth))

XCTAssertFalse(expandedSourceFile.hasError, "Expanded source should not contain any syntax errors")
Copy link
Member

Choose a reason for hiding this comment

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

What do you think about running ParseDiagnsoticGenerator to actually show the syntax errors?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds reasonable. I also considered showing the debug description but that seemed a bit much. This seems like a better idea.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this but IMO they still don't make much sense without the syntax tree. So I added that in as well.

@bnbarham bnbarham force-pushed the assert-on-macro-errors branch 2 times, most recently from e5ff9c1 to 41c0078 Compare June 12, 2023 20:30
@bnbarham
Copy link
Contributor Author

@swift-ci please test

Invalid macro expansions should generate an assertion in
`assertMacroExpansion` rather than being ignored.
@bnbarham bnbarham force-pushed the assert-on-macro-errors branch from 41c0078 to ea2d7e4 Compare June 12, 2023 21:37
@bnbarham
Copy link
Contributor Author

@swift-ci please test

@ahoppen
Copy link
Member

ahoppen commented Jun 13, 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.

4 participants