pgrx
pgrx copied to clipboard
Jcrist1 new generic tuples
I've recreated the PR as I've now got three crates on crates.io:
- tuple_tricks
- make_tuple_traits (a transitive dependency of the above, just to save me boilerplate)
- mark_tuple_traits a procedural macro necessary to bypass the
feature(fundamental)that would be required of the traits intuple_tricks.
As before this still implements a new method
pub fn get_tuple<(A1, A2, ...., An)>(&self) -> (Option<A1>, ..., Option<An>)
to the SpiHeapTupleData struct, that will try to get each type from that position.
In the repo for tuple_tricks I created an example to do a quick benchmark comparing a "hand rolled" implementation with the generic one, and the generic one was about 5% slower.
Edit: This is the new version of PR 279
@Hoverbear I pushed the crates to crates.io. This could probably be doable with the frunk library as well (but would still need to define a recursive trait), but couldn't figure it out quickly.
I forgot to apply the --release flag to my benchmark. There's now no difference in speed (and it was 100 times faster that debug build)
Thanks!
I need to think about this PR a bit more... Part of me is conflicted and thinks that it "might be less maintenance/mental effort to 'just' manually implement 32 From impls, also, proc macros are slow and we have too many already!" ... I'd like to convince it otherwise.
I was messing around with frunk, and came up with an implementation for hlists instead. The trait shenanigans are much simpler with hlists. And it gets rid of anything like the mark_tuple_traits! proc_macro. But the UX seems clunkier as then end users need to know about HList!, hlist! and hlist_pat! macros. It compiles but I can't get the test to work (it seems to crash pretty bad 😬) comparison here.
Looking at the definition of the HList! macro, it seems like it might be possible to implement the mark_tuple_traits! with a macro_rules!. Would that be better than a proc_macro?
Manual implementations would be easier, (and would give the most ergonomic ux), but 32 is overkill, and I just picked that number cause I could. But it would still be nice to have more than the get_three get_n::<A1, ..., An>. Maybe n <= 10? But I'm not sure the maintenance would be easier, as then one might need to make at least n changes.
Anyhow I'm happy to discuss this further, or yeet this if the prospective maintenance seems unsustainable / the compile time degrades too much. It was fun just coming up with a solution 😄.
It's an incredibly cool solution. :) I really like it. I just worry about 8 months from now going "What???" at it.
32 is kinda the typical (many std traits only work up to 32, for example), so might as well follow along.
You're the best judge of what kind of maintenance load this'll be. And when I proposed this I knew what the risks were I just did it cause I though it would be a cool solution so glad to hear 😄. I can close this, and if there's more interest in this kind of feature at a later date to justify the potential maintenance effort y'all can re-animate the code in the PR (How long do closed PRs stick around?). Also I'm not in a rush to delete my branch in my fork, either. Thumbs up if that sounds good?
Let's leave it open a bit. :) It's a feature I do want, and you did a great job, I just want to make sure we pick the simplest solution and I haven't had time to dig my fingers into it yet.