scylladb / cql-stress

10 stars 4 forks source link

Reworked to use a more intuitive procedural macro #35

Closed wprzytula closed 1 year ago

wprzytula commented 1 year ago

This makes usage more idiomatic - Rustaceans are used to putting such attribute macros above some structs.

piodul commented 1 year ago

After thinking about it more, I'd rather not use a procedural macro. Yes, #[derive(Runnable)] looks more natural than make_runnable!(StructName), but that's the only benefit. On the other hand, if we release cql-stress on crates.io, then the usage of procedural macros will force us to publish the runnable_macro crate, too as there is no way for a published crate to depend on a non-published crate. This will increase maintenance burden as there are two crates to maintain now, and we unnecessarily pollute the global namespace (we would have to change the name to e.g. cql-stress-macros, but that's still ugly IMHO).

Procedural macros are good for things that need to do some non-trivial transformation of the code and/or perform complicated calculations which are hard to express with the usual macro syntax. In case of Runnable, the impl basically looks always the same and you only need to change the StructName in the impl Runnable for StructName header. The non-procedural macro syntax is a good fit for something like this.

Ultimately, when (if?) the Rust language gains async methods with traits without boxing, we will be able to get rid of the make_runnable! hack altogether. If we publish the runnable_macro crate then we won't be able to unpublish it.