Made geyser interface repr(c) to improve interface stability #33703
Conversation
Codecov Report
@@ Coverage Diff @@
## master #33703 +/- ##
=======================================
Coverage 81.8% 81.8%
=======================================
Files 806 806
Lines 217676 217676
=======================================
+ Hits 178197 178208 +11
+ Misses 39479 39468 -11 |
|
Do we expect users to have to do much to accommodate this change in their plugins? |
One Last Recompile ™️ |
| @@ -112,6 +115,7 @@ pub struct ReplicaAccountInfoV3<'a> { | |||
| /// If there were a change to the structure of ReplicaAccountInfo, | |||
| /// there would be new enum entry for the newer version, forcing | |||
| /// plugin implementations to handle the change. | |||
| #[repr(C)] | |||
There was a problem hiding this comment.
we might consider setting the discriminant width on enums explicitly. they support a syntax like #[repr(C,u{8,16,32,64})] which corresponds to similar annotations in recent C++. afaik there's no such syntax in C though
There was a problem hiding this comment.
I made them repr(u32). Thanks.
There was a problem hiding this comment.
they'd need to be repr(C, u32), but repr(C) uses u32 discriminates by default iirc. so just repr(C) would be the same.
fwiw there's some annoying lint around multiple repr in current rust. they're only valid for data-bearing enums. given that these structs are not intended to be persisted or sent over the wire, repr(C) is probably actually the least wrong choice
There was a problem hiding this comment.
I tried repr(C, u32), it does not compile. According to https://doc.rust-lang.org/nomicon/other-reprs.html, it should be repr(u*) format. repr(C), also:
repr(C) is equivalent to one of repr(u*) (see the next section) for fieldless enums. The chosen size is the default enum size for the target platform's C application binary interface (ABI). Note that enum representation in C is implementation defined, so this is really a "best guess". In particular, this may be incorrect when the C code of interest is compiled with certain flags.
So I think repr(u32) is more explicit. https://github.com/rust-lang/rfcs/blob/master/text/2195-really-tagged-unions.md documented more benefits on enum with fields.
Yeah, that's what I was hoping :) |
I have not found it requires any code change needed from the plugin side. Just compile and it works. |
|
@t-nelson @CriesofCarrots , a gentle nudge on this PR. |
Problem
Spurious validator crashes due to rust version mismatch for validator and plugins.
Summary of Changes
Made geyser interface repr(c) to improve interface stability
Fixes #