Skip to content

Clean code gen #1343

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.

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 5 commits into from
Feb 14, 2023
Merged

Clean code gen #1343

merged 5 commits into from
Feb 14, 2023

Conversation

kimdv
Copy link
Contributor

@kimdv kimdv commented Feb 13, 2023

This is a PR that grew a bit. If it need to be split, I'll do so!

I think it's easier to review commit by commit.

@kimdv
Copy link
Contributor Author

kimdv commented Feb 13, 2023

Maybe we should create a little tool for rewriting the headers 😎

@kimdv kimdv force-pushed the kimdv/sort-gyb-files branch 2 times, most recently from bb725fb to 1b8d93b Compare February 14, 2023 08:06
Copy link
Member

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

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

Thanks. Could you extract the commit that removes force try to a separate PR because I’ve got opinions on that one. All the other commits are looking good to me and we can merge them in one PR.

Regarding force try: Instead of throwing, I think we should fatalError as close to the point where the syntax error was introduced as possible instead of throwing an error from the main function because that makes fixing the bugs easier. My vision for this has been that:

  1. For each function that throws (i.e. the ones in SyntaxNodeWithBody.swift) we add a variant in CodeGeneration that doesn’t throw but has a label on the PartialSyntaxNodeString parameter that’s named validating or something of the sort. When the original function throws, this function fatalErrors
  2. In CodeGeneration we add an initializer init(validating node: Self) to SyntaxProtocol. This initializer fatalErrors whenever node has a syntax error. And we’d use this initializer whenever we are creating a syntax node from string interpolation in CodeGeneration

That way we would restore the behavior prior to #1290 in CodeGeneration because that kind of behavior was actually really nice for code generation IMO.

@kimdv kimdv force-pushed the kimdv/sort-gyb-files branch from 1b8d93b to a659902 Compare February 14, 2023 11:49
@kimdv kimdv force-pushed the kimdv/sort-gyb-files branch from a659902 to 31368c4 Compare February 14, 2023 11:53
@kimdv kimdv requested a review from ahoppen February 14, 2023 12:02
@kimdv
Copy link
Contributor Author

kimdv commented Feb 14, 2023

@swift-ci please test

@kimdv kimdv merged commit c1d6b43 into swiftlang:main Feb 14, 2023
@kimdv kimdv deleted the kimdv/sort-gyb-files branch February 14, 2023 15:15
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