gtk-rs-core
gtk-rs-core copied to clipboard
RFC: glib-macros: port attribute parsing to use deluxe crate
I have this crate that can generate parsers for attributes using a derive macro. It automatically throws errors (and spelling suggestions) on invalid attributes. We can get better errors messages and remove some code this way so here is a port. The tests are passing for me without any changes.
Also made some changes to avoid erroring out early. Before it would just stop on first error, now any compile errors from inner attributes like #[enum_value(name = ...)] will be aggregated and included in the final output.
Fixes #943
I'm not very convinced by the added value. It makes the code more complicated to understand and adds more proc-macro.
Yes, this was more needed for the class macro where there are lot of attributes. And the properties macro too if wanted.
The idea though is to make it so actually running the proc macro is faster due to the generated parser, but I have not done any benchmarks yet.
Might be better to have some numbers indeed (not just for this PR).
btw @ranfdev I will be using this for the class macro and it will be duplicating the same attributes from the properties macro
@GuillaumeGomez PR now updated with a test to see how porting of one of the wrapper macros to a proc macro would be like
Some thinking here: the current syntax of the wrapper is really unfortunate. It takes a tuple struct but then it turns it into a normal struct with a single field inner. If we want to add other fields to it they can't be added without using another awkward syntax. So I turned it into an attribute macro that works on any struct, it always takes the first field and infers the type from it. Also now you can turn off generation of any trait to manually implement it.
Also now includes a port of clone_block to use attributes as a replacement for clone! and closure! @Hofer-Julian
The number of changes is impressive.
I agree with @GuillaumeGomez that we should evaluate if it's worth porting everything to this macro or not.
Mainly because contributing to glib-macros would then require another layer of knowledge: knowing the deluxe macro.
For example, if the properties macro gets rewritten, I would need a bit of time to learn deluxe to get comfortable working on the properties macro again.
I think it would be better to keep the number of changes to a minimum, so that we can better evaluate the changes introduced by using deluxe.
Unless clone_block is strictly required to port the macros to deluxe, it's probably better if we introduce a separate PR for that.
These are some changes I have had in the back burner for at least several months now in various states of completeness... clone_block is not needed, all of them are separate. I've just been putting all my macros together in this PR for now. Note only the parsing gets rewritten here, the main reason I am doing this is:
- To remove
syn::parse::Parseimpls and equivalents and just autogenerate them. The token output stays the same. Except for the wrapper macro, that one was rewritten when ported to a proc macro. - To standardize the error messages. I have done a lot of work on deluxe to make it more like darling where parsing can continue after an error and the errors are all aggregated, and you get spelling suggestions on unknown attributes; currently all the existing macros just exit on the first parse error.
- To standardize on normal rust syntax to make it easier to use and to make clippy and rustfmt work again, those will probably never work correctly with function-like macros.
An alternative here is to try and bring back some of the utils and techniques from darling/deluxe into glib-macros and implement them manually that way, but my hope is the autogenerated parsers are less error prone
Also now includes a port of
clone_blockto use attributes as a replacement forclone!andclosure!@Hofer-Julian
This is great! I am not familiar with the codebase, so I cannot judge the diff. However, from a user perspective, the clone_block would be greatly appreciated. Many users work around that they cannot use clippy/rustfmt with closures inside clone! and some like @sdroege don't use clone! at all. clone_block would solve that.
Working on benchmark here https://github.com/jf2048/glib-macro-bench
As I expected enum_derive is a bit faster now but the wrapper is slower...