private-attribution / ipa

A raw implementation of Interoperable Private Attribution
MIT License
42 stars 25 forks source link

[Draft] Macro for serialization-related trait bounds #1427

Closed tyurek closed 1 week ago

tyurek commented 2 weeks ago

First attempt at making a macro for generating some of the trait bounds. The interface is rather clunky as we can't just put a macro invocation in the trait bounds section. I'm following the pattern from this SO post: https://stackoverflow.com/questions/68728105/how-can-i-generate-trait-bounds-in-a-declarative-macro

The committed code has an example of what this would look like, which I'll also reproduce here:

with_bounds_for! { BK, ( $($bounds:tt)* ) => (
impl<BK: SharedValue> Serializable for HybridImpressionReport<BK>
where
    // Additional bounds could go here
    $($bounds)*
{
    type Size = gen_size!(BK);
   // proceed to define whatever functions...
}
)}

If we think this is still worthwhile, then I can expand this and start replacing some of the trait bounds, but I'm worried about making things clunkier, so I'd like a second opinion or two.

Alternatively, I tried using serde, but I'm stuck at making AdditiveShare work with serde::Serialize/Deserialize. I'm not sure how to implement these traits for <V as vector::traits::Vectorizable>::Array

codecov[bot] commented 2 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 93.28%. Comparing base (04ba0a7) to head (895bad1). Report is 58 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1427 +/- ## ========================================== + Coverage 93.25% 93.28% +0.02% ========================================== Files 224 225 +1 Lines 38535 38717 +182 ========================================== + Hits 35937 36117 +180 - Misses 2598 2600 +2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.


🚨 Try these New Features:

eriktaubeneck commented 2 weeks ago

when you only have one generic type (in this example, BK), the trait bound isn't too crazy. where it get's complicated is when you start adding more and more generics.

this approach is a good start, but we actually need to make it recursive, based on an array of generics. a good start for this one would be to make BA64 generic as well (e.g., MK), and make this work recursively.

tyurek commented 2 weeks ago

when you only have one generic type (in this example, BK), the trait bound isn't too crazy. where it get's complicated is when you start adding more and more generics.

this approach is a good start, but we actually need to make it recursive, based on an array of generics. a good start for this one would be to make BA64 generic as well (e.g., MK), and make this work recursively.

Yeah, the macro itself will need some work, but do you think the interface for calling it is acceptable?

eriktaubeneck commented 2 weeks ago

Yeah, the macro itself will need some work, but do you think the interface for calling it is acceptable?

Not quite. It should take an array of generics, not a single one. For example, you need to be able to build this with [MK, BK, V].

akoshelev commented 2 weeks ago

is it something where a proc macro can help?