Skip to content
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

api-macros: Move request / response trait impl generation into a custom derive #664

Closed
wants to merge 1 commit into from

Conversation

jplatte
Copy link
Member

@jplatte jplatte commented Jul 7, 2021

Once completed, this should fix the issue identified in #663 (comment) by making the compiler expand cfgs before these trait implementations are generated.

Thanks @DevinR528 for analyzing why #663 causes issues!

@jplatte jplatte requested a review from DevinR528 July 7, 2021 16:06
}

impl<'ast> Visit<'ast> for Visitor {
fn visit_lifetime(&mut self, _lt: &'ast Lifetime) {
Copy link
Member

Choose a reason for hiding this comment

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

Sweet I had changed this also while looking for the problem!

Copy link
Member

@DevinR528 DevinR528 left a comment

Choose a reason for hiding this comment

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

Wow, I spent hours trying to find this bug I think I just found it. https://github.com/ruma/ruma/blob/main/crates/ruma-api-macros/src/api/request/incoming.rs#L181 just need to add the cfg there then I think that's it?

@jplatte jplatte force-pushed the conditional-lifetime-decls branch 6 times, most recently from 5d4bd62 to 5d54262 Compare July 10, 2021 18:49
@jplatte jplatte changed the title api-macros: Make lifetime declarations cfg-conditional if uses are api-macros: Move request / response trait impl generation into a custom derive Jul 16, 2021
@jplatte jplatte marked this pull request as draft July 16, 2021 10:34
@jplatte jplatte force-pushed the conditional-lifetime-decls branch from 5d54262 to f17a1b9 Compare July 16, 2021 20:34
@jplatte
Copy link
Member Author

jplatte commented Jul 16, 2021

Made a decent amount of progress but there's still a lot to do. I'll also have to benchmark this, it could increase compile times of the api crates due to more parsing and general back-and-forth between the proc-macro and the compiler.

@jplatte
Copy link
Member Author

jplatte commented Jul 27, 2021

Code structure are starting to make sense again in ruma-api-macros, but there's still a bit of work left. Hopefully I can finish this over the next few weeks.

@jplatte jplatte force-pushed the conditional-lifetime-decls branch 4 times, most recently from 263a143 to b1c89ff Compare August 5, 2021 11:29
@jplatte
Copy link
Member Author

jplatte commented Aug 5, 2021

One more lifetime issue to figure out, then this should be ready!

@jplatte jplatte marked this pull request as ready for review August 5, 2021 18:32
@jplatte
Copy link
Member Author

jplatte commented Aug 5, 2021

Merged on the command line.

@jplatte jplatte closed this Aug 5, 2021
@jplatte jplatte deleted the conditional-lifetime-decls branch August 5, 2021 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants