Repository navigation
Update and add support for uniffi v0.31.2 - #2
Conversation
alan-george-lk
changed the title
cb233a9 to
cf5ac7c
Compare
|
|
||
| pub(crate) fn literal_cpp( | ||
| literal: &Literal, | ||
| default: &DefaultValue, |
There was a problem hiding this comment.
nit, default_value might be better name here ?
| while !definition_topology.peek_all().is_empty() { | ||
| let list = definition_topology.pop_all(); | ||
| let mut list = definition_topology.pop_all(); | ||
| list.sort(); |
There was a problem hiding this comment.
any technical reason why we need list.sort() here ?
There was a problem hiding this comment.
This was a technically unrelated fix, I could move off this branch if you prefer.
My understanding is pop_all() does not guarantee order due to hashing, so without this list.sort() generated code could be non-deterministic in order.
Would not affect a lot, but I know there are some C++ nuances around member initialization order/lifetimes so this is a good practice I'd argue.
jhugman
left a comment
jhugman
left a comment
There was a problem hiding this comment.
This looks good from a uniffi and Rust pov. It's a real pleasure looking at small bindgen.
It may be further up in the stack, but I'm not seeing:
- methods on records and enums
- uniffi traits on the same.
(these are the largest omissions by comparing against the uniffi-rs changelog for the version range).
These omissions may be better done in another PR, so I'm going to r+ this one so as not to hold you up.
|
|
||
| let rest = types | ||
| .filter(|&t| !sorted.contains(t)) | ||
| .filter(|t| type_name(t).map_or(true, |name| !emitted_names.contains(name))) |
| ASSERT_EQ(first.to_debug_string(), "TraitRecord { s: \"same\", i: 1 }"); | ||
| ASSERT_TRUE(first.eq(second)); | ||
| ASSERT_FALSE(first.ne(second)); | ||
| ASSERT_FALSE(first.eq(different)); | ||
| ASSERT_EQ(first.hash(), second.hash()); | ||
| ASSERT_EQ(first.cmp(second), 0); |
There was a problem hiding this comment.
I was skeptical of these method names vs. operators and other options in C++, but there is precedent for them in this bindgen/Rust already for objects
| int8_t cmp({{ type_name }} _uniffi_self, {{ type_name }} other) { | ||
| return uniffi::{{ Type::Int8.borrow()|lift_fn }}({% call macros::rust_call_with_value(method, typ) %}); | ||
| } | ||
| {%- endif %} |
| {%- if uniffi_trait_methods.ord_cmp.is_some() %} | ||
| /** Calls the value's Rust `Ord` implementation. */ | ||
| int8_t cmp({{ type_name }} _uniffi_self, {{ type_name }} other); | ||
| {%- endif %} |
3 participants
This PR updates the C++ bindgen from UniFFI
0.29.4to0.31.2.Required compatibility changes include:
uint64_thandlesuniffi_clone, odd/even handle ownership, and round-tripping Rust- and C++-implemented trait objectsOrd::cmptrait method#[uniffi::trait_interface]0.31.2Debug,Display,Eq,Hash, andOrdtraits on record and enum value types