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

move compiler plugins to proc macro #156

Closed
dovahcrow opened this issue Jan 28, 2017 · 7 comments
Closed

move compiler plugins to proc macro #156

dovahcrow opened this issue Jan 28, 2017 · 7 comments
Labels
enhancement A minor feature request

Comments

@dovahcrow
Copy link

dovahcrow commented Jan 28, 2017

Now that proc_macto_attribute has landed, it would be nicer to migrate from old compiler plugin based codegen to the proc_macro.

I'm working on an attempt on my fork using pure proc_macro, syn and quote.

It's just a notice in case of duplicated work from someone else.

@dovahcrow dovahcrow changed the title move compiler plugins to proc_macro move compiler plugins to proc macro Jan 28, 2017
@SergioBenitez SergioBenitez added the enhancement A minor feature request label Jan 29, 2017
@SergioBenitez
Copy link
Member

Indeed! Landed in rust-lang/rust@375cbd2, it seems.

While I'm very excited about your eagerness to contribute, I believe it's a bit early to make this transition. The proc_macro feature went through quite a bit of change after landing, and those changes impacted the consumers of libraries using it. I'd rather avoid that kind of impact to Rocket's users. I think we should wait a bit on this.

Nonetheless, I think it's more than prudent to begin exploring migrating Rocket to the new feature. If there are bumps along the way, or things that simply cannot be done with the new API, I'm sure the Rust team would love to be made aware.

@dovahcrow
Copy link
Author

After consideration, I decided to make it a separate library that coexists with rocket_codegen.

Benefits are that user can decide which library to use and fallback to the much "stable" rocket_codegen if they seek for no breaking change and since the codegen part is the biggest obstacle of migrating rocket to stable Rust, a proc_macro library after proc_macro_attribute landed in stable rust can drive rocket a big leap toward stable rust.

@SergioBenitez
Copy link
Member

Please don't do this. Don't release a library as an alternative to rocket_codegen. This will simply confuse users. rocket_codegen is not static, and neither is rocket. Both are changing often. Features are being added/changed to/in codegen often; any differences between codegen and your library will result in confusion at best and breakage at worst. Further, the interface between core and codegen is not visible to users, and likely will never be, so your library can break without notice, breaking user's applications.

I absolutely encourage you to experiment, and even submit a PR if things come along far enough, but please don't release something that can lead to confusion. There's absolutely no advantage; custom_derive will be around for the foreseeable future, and when proc_macro_attribute matures a bit, Rocket will wholeheartedly migrate to it.

@dovahcrow
Copy link
Author

dovahcrow commented Jan 29, 2017 via email

@dovahcrow
Copy link
Author

dovahcrow commented Jan 29, 2017

I've just implement almost all the things including derive, route and error catcher but not route! and errors! macro since the #[proc_macro] has not landed yet. I'll implement the rest as soon as the feature landed.

@SergioBenitez
Copy link
Member

Closing this issue for now. We certainly want to move in this direction, but error reporting from proc_macro_attribute is simply not there yet. Additionally, this is part of #19.

@dovahcrow
Copy link
Author

Yes, since rocket has detailed error reporting after this issue initiated, I think it is not possible to close this issue in the near future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A minor feature request
Projects
None yet
Development

No branches or pull requests

2 participants