Skip to content

Move codegen postprocessing to its own module #2282

New issue

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

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

Already on GitHub? # to your account

Merged
merged 9 commits into from
Sep 27, 2022

Conversation

pvdrz
Copy link
Contributor

@pvdrz pvdrz commented Sep 23, 2022

This PR moves all the syn based postprocessing to its own module inside codegen.

@pvdrz pvdrz requested review from amanjeev, emilio and kulp and removed request for amanjeev and emilio September 23, 2022 20:29
Copy link
Contributor

@emilio emilio left a comment

Choose a reason for hiding this comment

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

This looks great, thanks!

I have one suggestion that avoids some of the macro soup, let me know what you think. I implemented it in 0798bda.

I think it's a bit easier to follow, if you agree feel free to cherry-pick that on top before merging.

Thanks again!

@pvdrz
Copy link
Contributor Author

pvdrz commented Sep 26, 2022

I have one suggestion that avoids some of the macro soup, let me know what you think. I implemented it in 0798bda.

@emilio, that's a pretty cool pattern! I'm going to steal it 😬

I did some changes to your commit so PASSES is a const slice so we don't have to update its size when adding a new option. I'm a bit confused about the TODO comment on top of the pass macro. How could you make that as a const fn?

Copy link
Member

@amanjeev amanjeev left a comment

Choose a reason for hiding this comment

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

This looks lovely!

@pvdrz pvdrz merged commit b5ec18e into rust-lang:master Sep 27, 2022
@pvdrz pvdrz deleted the sovereign-module-of-syn branch September 27, 2022 19:11
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants