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

Refactor the derive crate #63

Open
ascjones opened this issue Feb 3, 2021 · 4 comments
Open

Refactor the derive crate #63

ascjones opened this issue Feb 3, 2021 · 4 comments

Comments

@ascjones
Copy link
Contributor

ascjones commented Feb 3, 2021

As raised in #61 (review), it is less than ideal we have to pass around the crate names to all the functions.

Originally the proc macro was very small so could justify being a few functions, but now it is becoming unwieldy and there is the above mentioned problem of shared global state.

One approach would be to separate the process into 2 steps parse -> Intermediate Representation -> expand which is a common pattern in proc macros.

@Robbepop
Copy link
Contributor

Robbepop commented Feb 3, 2021

You might be thinking about using the https://crates.io/crates/synstructure crate for the refactoring of the derive macros.
In ink! we are using this crate and it really simplifies a lot and was also a great learning experience since it heavily simplifies the concepts behind the syntactic structures behind Rust code.

@ascjones
Copy link
Contributor Author

ascjones commented Feb 3, 2021

I did experiment with that and decided (at the time) that it wasn't suitable because it was more aimed at generating derived impls for type instances, and we have a static method so no instance to match against.

However I could be wrong and it is certainly worth revisiting.

@Robbepop
Copy link
Contributor

Robbepop commented Feb 3, 2021

I did experiment with that and decided (at the time) that it wasn't suitable because it was more aimed at generating derived impls for type instances, and we have a static method so no instance to match against.

However I could be wrong and it is certainly worth revisiting.

It is a very flexible crate, yet also introduces further complexity because you have to understand how it is meant to be used. So please take my thoughts as reminder that this crate exists and might do a good job here, not as suggestion. ;)

@ascjones
Copy link
Contributor Author

ascjones commented Feb 3, 2021

I will definitely look again at it - I do like what I have seen where you have used it.

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

No branches or pull requests

2 participants