Skip to content

Draft: Try out derive-adhoc for tor-linkspec's RelayIdRef

Ian Jackson requested to merge Diziet/arti:da-relayid into main

(This is not actually a proposal to merge anything into Arti. It's just a convenient way to communicate with @nickm about derive-adhoc.)

This is a reworking of RelayId and RelayIdRef to use a (truly-adhoc) derive-adhoc template.

@nickm, you want to read just the top commit, since the branch is on top of !1156 (merged).

Things that seemed nice:

  • Bugs of the form match ... { X => BlarkXThing, Y => BlarkXThing, } are made impossible in the treated functions. This is really nice since such idioms are a major source of real-world bugs; particularly in Rust with rustfmt, where we can't use tabular layout.

  • Adding a new key type would involve many fewer changes (and one couldn't forget the PartialEq impls).

  • There was no problem having some bespoke methods alongside the systematic macro-generated ones.

  • unwrap_* is deduplicated.

  • The dbg debugging feature worked nicely and solved a problem I had.

Things that are slightly vexing:

  • The repetition of ${paste $vname Identity} is vexing. A shorter syntax for ${paste } (as proposed in would partly mitigate this. What's really needed is internal macro macros, as requested in Diziet/rust-derive-adhoc#14

  • Making all of this code part of a macro invocation hides it from rustfmt. And, strictly speaking, the whole macro body ought to get another indent level.

Trouble now resolved:

  • My first cut failed the tests because I had omitted .. as lit on the use of ${meta(display)}. See the code comment about this. I had to use the dbg feature to diagnose it. Bug fixed in derive-adhoc 0.4.

  • I wasn't able to faithfully reproduce the documentation because std's stringify! doesn't work in #[doc]. I think this can't be done without a ${string } in derive-adhoc. I just had the wrong syntax; with the right syntax it works fine.

  • I ended up using $vtype when I needed the value constructor for one of the unit variants of the driver, RelayIdType. This is a bit weird, although it generates output which is correct for a unit. $vpat would have worked too. This doesn't seem well-documented in the reference. Probably we should recommend $vpat, but I'm not sure. The d-a reference does say "To construct a value, prefer $vtype to $ttype" which legitimises this use but I think $vpat would be fine too.

Edited by Ian Jackson

Merge request reports