✨ zv: Added a signature! macro #1536

hadihaider055 · 2025-10-09T18:23:25Z

Add a signature! macro that creates const signatures with compile-time validation. This makes it easier to work with D-Bus signatures while catching invalid ones early.

Fixes #984

zeenix · 2025-10-09T19:45:56Z

@hadihaider055 thanks! I'll have a look later but please remember to use emoji rather than the emoji code. 😉

hadihaider055 · 2025-10-09T20:03:14Z

Ah sure @zeenix, I just saw this on mobile and it seems like the emoji code isn't working. Thanks for pointing it out!

zeenix · 2025-10-09T22:21:28Z

Ah sure @zeenix, I just saw this on mobile and it seems like the emoji code isn't working. Thanks for pointing it out

It won't work in many software (including git cli tools). That's why it's best to follow the advice in the contribution guidelines exactly. 😉

zeenix

Super cool. 👍 I've been meaning to do this for a while now. :) Just two things apart from the inline comments:

It's a bit unexpected to have a non-derive macro in a crate suffixed _derive. I think, it possible, we should add this in zvariant_utils and then re-export from zvariant.
Let's make use of this in the zvariant tests where we create signature from a static string (there are at least 3 places, I can see).

zvariant_derive/src/type.rs

zvariant_derive/src/lib.rs

codspeed-hq · 2025-10-17T04:45:11Z

CodSpeed Performance Report

Merging #1536 will not alter performance

_{Comparing hadihaider055:main (6befcae) with main (833dd31)}

Summary

✅ 22 untouched

hadihaider055 · 2025-10-17T05:06:40Z

@zeenix, I have moved the implementation to the zvariant_utils and re-exported it from the zvariant. Fixed all the issues related to pub(crate), and the impl is split and moved to a separate file in zvariant_utils.

zeenix · 2025-10-17T10:51:32Z

@zeenix, I have moved the implementation to the zvariant_utils and re-exported it from the zvariant. Fixed all the issues related to pub(crate), and the impl is split and moved to a separate file in zvariant_utils.

Thanks for doing that. 👍 However, the macro still remains in zvariant_derive. The idea was that it doesn't belong there, so it should only be zvariant that rexports it and the whole macro (not just the impl) should be in zvariant_utils. Having said that, there are some strange rules about where macros could be and can't be, so it's possible that those rules don't allow a macro to be in zvariant_utils. If so, we'll have to live with it being in zvariant_derive (even if it's strange). You should be able to very quickly figure that out when you try to move the macro there.

BTW, the PR title and commit title say "Siganture", while the macro is called "signature".

hadihaider055 · 2025-10-17T16:28:40Z

Thanks for doing that. 👍 However, the macro still remains in zvariant_derive. The idea was that it doesn't belong there, so it should only be zvariant that rexports it and the whole macro (not just the impl) should be in zvariant_utils. Having said that, there are some strange rules about where macros could be and can't be, so it's possible that those rules don't allow a macro to be in zvariant_utils. If so, we'll have to live with it being in zvariant_derive (even if it's strange). You should be able to very quickly figure that out when you try to move the macro there.

Thanks @zeenix! You're right on moving the entire macro to zvariant_utils, but there's a Rust compiler limitation: procedural macros can only be defined in crates with proc-macro = true in their Cargo.toml.

Currently, zvariant_derive has proc-macro = true; however, the zvariant_utils does not (it's a regular library crate). I've already moved the implementation logic to macro_impl.rs and the macro in lib.rs just has a thin wrapper that calls it.

For now, I think we have 2 options:

Keep the macro wrapper in zvariant_derive (current approach).
Convert zvariant_utils to a proc-macro crate, but this would prevent it from being used as a regular library
dependency.

What do you think is the best approach? I believe option 1 is the most practical since most of the logic is already in zvariant_utils.

BTW, the PR title and commit title say "Siganture", while the macro is called "signature".

Thanks for pointing it out! I'll fix this.

zeenix · 2025-10-17T18:32:19Z

Thanks @zeenix! You're right on moving the entire macro to zvariant_utils, but there's a Rust compiler limitation: procedural macros can only be defined in crates with proc-macro = true in their Cargo.toml.

Yeah, that's what I feared. 😢

Keep the macro wrapper in zvariant_derive (current approach).

As I wrote in my previous comment, that's what we have to do then. Then there is also no need for the impl to be in _utils since that crate is mainly to keep common code between zvariant_derive and zbus_macros and zvariant API that we need in the macro code itself but can't be in zvariant_derive itself.

What do you think is the best approach? I believe option 1 is the most practical since most of the logic is already in zvariant_utils.

Yeah but we don't want the impl in _utils for the reason I mentioned.

zeenix

LGTM otherwise. Thanks so much!

We should advertise this signature a bit more. E.g the Signature docs should recommend this macro and make use of that in its examples. Also grep for uses of Signature::try_from and Signature::from_str in the repo and update them. Please keep the atomic commits preference in mind, while at it. ;)

zeenix · 2025-10-19T11:02:48Z

zvariant_derive/src/lib.rs

+/// Allows to create a `Signature` from a string literal at compile time,
+/// with validation to ensure the signature string is valid.
+///
+/// The macro validates the signature string at compile time:


We need examples here.

This could just be a copy of the examples in the Signature docs itself (after you've converted them to use this macro) + a const example (just copy from the commit log).

zvariant_derive/src/signature.rs

zeenix · 2025-10-21T10:47:31Z

Please keep the atomic commits preference in mind, while at it. ;)

☝️

hadihaider055 · 2025-10-21T10:48:00Z

@zeenix, I've added the documentation with proper examples in the Signature macro and updated the Signature type docs to recommend it. Let me know if there's anything unclear for the readers.

Implement a `signature!` macro that constructs const Signatures with compile-time validation. The macro allows creating Signature from string literals at compile time: const SIGNATURE: Signature = signature!("a{sv}"); let sig = signature!("yys"); Fixes: z-galaxy#984

hadihaider055 · 2025-10-21T11:07:48Z

Please keep the atomic commits preference in mind, while at it. ;)

@zeenix, on it! Should I separate it into 3 commits with the feature, docs, and testing? Or rather, the tests be committed with the feature commit ✨ zv: Added a signature macro.

zeenix · 2025-10-21T12:25:33Z

@zeenix, on it! Should I separate it into 3 commits with the feature, docs, and testing? Or rather, the tests be committed with the feature commit ✨ zv: Added a signature macro.

Both options are fine but 3 commits would be better indeed.

Recommendation for the signature! macro and examples about the runtime and compile-time signature creation.

Replace Signature::try_from() and Signature::from_str() calls with the new signature! macro.

hadihaider055 · 2025-10-21T13:01:13Z

@zeenix, I've split the changes into 3 commits (feature, doc, and test). Is it fine?

hadihaider055 force-pushed the main branch from f9839a2 to 71c6a43 Compare October 9, 2025 18:45

hadihaider055 changed the title ~~✨ zv: Added a Signature Macro~~ ✨ zv: Added a Signature Macro Oct 9, 2025

hadihaider055 force-pushed the main branch from 71c6a43 to 8f0e524 Compare October 9, 2025 20:05

zeenix requested changes Oct 11, 2025

View reviewed changes

zvariant_derive/src/type.rs Outdated Show resolved Hide resolved

zvariant_derive/src/lib.rs Outdated Show resolved Hide resolved

hadihaider055 force-pushed the main branch from 8f0e524 to e8ebb64 Compare October 17, 2025 04:41

hadihaider055 changed the title ~~✨ zv: Added a Signature Macro~~ ✨ zv: Added a signature! macro Oct 17, 2025

hadihaider055 force-pushed the main branch from e8ebb64 to 305820f Compare October 18, 2025 13:59

zeenix requested changes Oct 19, 2025

View reviewed changes

hadihaider055 force-pushed the main branch 2 times, most recently from cb7d010 to fcf35d7 Compare October 21, 2025 10:38

hadihaider055 added 2 commits October 21, 2025 17:40

📝 zv: Document signature! macro and update Signature docs

601cd7e

Recommendation for the signature! macro and examples about the runtime and compile-time signature creation.

♻️ zv: Use signature! macro in tests

6befcae

Replace Signature::try_from() and Signature::from_str() calls with the new signature! macro.

hadihaider055 force-pushed the main branch from fcf35d7 to 6befcae Compare October 21, 2025 12:49

Uh oh!

✨ zv: Added a signature! macro #1536

Are you sure you want to change the base?

✨ zv: Added a signature! macro #1536

Conversation

hadihaider055 commented Oct 9, 2025

Uh oh!

zeenix commented Oct 9, 2025

Uh oh!

hadihaider055 commented Oct 9, 2025

Uh oh!

zeenix commented Oct 9, 2025

Uh oh!

zeenix left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

codspeed-hq bot commented Oct 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CodSpeed Performance Report

Merging #1536 will not alter performance

Summary

Uh oh!

hadihaider055 commented Oct 17, 2025

Uh oh!

zeenix commented Oct 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hadihaider055 commented Oct 17, 2025

Uh oh!

zeenix commented Oct 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zeenix left a comment

Choose a reason for hiding this comment

Uh oh!

zeenix Oct 19, 2025

Choose a reason for hiding this comment

Uh oh!

zeenix Oct 19, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

zeenix commented Oct 21, 2025

Uh oh!

hadihaider055 commented Oct 21, 2025

Uh oh!

hadihaider055 commented Oct 21, 2025

Uh oh!

zeenix commented Oct 21, 2025

Uh oh!

hadihaider055 commented Oct 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codspeed-hq bot commented Oct 17, 2025 •

edited

Loading

zeenix commented Oct 17, 2025 •

edited

Loading

zeenix commented Oct 17, 2025 •

edited

Loading