extract codegen into macro-rules macros #185
Conversation
The procedural macro now generates a call to `setup_class!` which generates the actual code.
We used to process types in many ways and pass in a number of various. We now generate most of those in declarative macro-rules logic in `java_types.rs` so that we can just pass in token tree description of the java type and let the rest of the code work.
This lets us avoid the grodiness of having to pass in the self identifier.
There was a problem hiding this comment.
This is definitely a big improvement in terms of readability of the codegen.
In the long term, having some tests of the macros directly is probably a reasonable thing to add—I think it will make working on the macros easier.
There is some dead (and I think? incorrect) code in macro_if that we should remove before merging. But otherwise this looks good.
macro-rules/src/util.rs
Outdated
| (if [$($input:tt)+] { $($then:tt)* }) => { | ||
| $($then)* | ||
| }; |
There was a problem hiding this comment.
I'm kinda confused, but this looks like it ignores the $input arguments? AFAICT it expands identically to if true
There was a problem hiding this comment.
ah—I think actually all of these expect the last two are dead code
| @@ -68,6 +68,12 @@ pub mod plumbing { | |||
| pub use crate::raw::{EnvPtr, FieldPtr, FromJniValue, IntoJniValue, MethodPtr, ObjectPtr}; | |||
| pub use crate::refs::NullJRef; | |||
| pub use crate::to_java::{ToJavaImpl, ToJavaScalar}; | |||
| pub use duchess_macro_rules::{ | |||
There was a problem hiding this comment.
I wonder if we should make the whole plumbing module doc(hidden)
There was a problem hiding this comment.
Yes. I've been thinking we should also rename it to semver_unstable or something more explicit.
|
I'm of mixed minds about unit tests on the macros themselves — they don't represent a stable API boundary and I'm generally not in favor of tests that don't target a stable API boundary, since they can become dated so easily when refactorings occur. On the other hand, I know they can have value in making it easy to specify edge cases. |
|
Thanks for the review, @rcoh, I'll go over the docs and make a few improvements and then merge. |
makes it easier to find them. also, improve comments.
|
Made cosmetic improvements. Will merge once CI is green. |
This applies a pattern that I've found to be very nice to work with, in which the duchess crate has two helper crates:
duchess-macro-rules, internalmacro_rules!macro that handle most of the codegenduchess-macros, procedural macros which generate calls to those macros after doing validation and pre-processingThis combo lets each kind of macro manage the things it is good at: procedural macros for generating errors and doing some grody stuff, but macro-rules for generating the actual code.
Also, as a driveby change, adds a
dev-container.jsonfile which makes it easier to manage the setup for testing and developing.r? @rcoh