Skip to content

Commit ac9eaf1

Browse files
committed
Implement Lift for flat errors
We noticed a regression in `0.25.x` where dictionaries containing flat errors caused code not to be compiled. We never officially supported compound types that stored errors, but it seems better to allow the code to compile in this case. The reason this isn't supported is that we can't lift flat errors, so therefore we can't lift the dictionary. A better fix for this would be to not implement `Lift` on the dictionary in this case, which would cause a compile error rather than a runtime panic. However, that's actually not so simple as noted in the comment so I went for an easier way. This brings this very close to how they worked in `0.24.x` where we would generate an `FfiConverter` implementation for flat errors, but the lift half of it would just be panics.
1 parent c6ddcee commit ac9eaf1

File tree

3 files changed

+77
-3
lines changed

3 files changed

+77
-3
lines changed

fixtures/coverall/src/coverall.udl

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@ namespace coverall {
2222
void test_getters(Getters g);
2323

2424
sequence<string> ancestor_names(NodeTrait node);
25+
26+
ReturnOnlyDict output_return_only_dict();
27+
ReturnOnlyEnum output_return_only_enum();
2528
};
2629

2730
dictionary SimpleDict {
@@ -49,6 +52,21 @@ dictionary SimpleDict {
4952
NodeTrait? test_trait;
5053
};
5154

55+
// Create a type that stores `CoverallFlatError` and therefore can only be lowered but not lifted.
56+
// UniFFI should define a `Lower` implemenation but not try to define `Lift`.
57+
dictionary ReturnOnlyDict {
58+
CoverallFlatError e;
59+
};
60+
61+
// More complicated version of the above, each variant is return-only for different reasons
62+
[Enum]
63+
interface ReturnOnlyEnum {
64+
One(CoverallFlatError e);
65+
Two(ReturnOnlyDict d);
66+
Three(sequence<CoverallFlatError> l);
67+
Four(record<string, CoverallFlatError> m);
68+
};
69+
5270
dictionary DictWithDefaults {
5371
string name = "default-value";
5472
string? category = null;

fixtures/coverall/src/lib.rs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,37 @@ pub struct SimpleDict {
134134
test_trait: Option<Arc<dyn NodeTrait>>,
135135
}
136136

137+
pub struct ReturnOnlyDict {
138+
e: CoverallFlatError,
139+
}
140+
141+
pub enum ReturnOnlyEnum {
142+
One {
143+
e: CoverallFlatError,
144+
},
145+
Two {
146+
d: ReturnOnlyDict,
147+
},
148+
Three {
149+
l: Vec<CoverallFlatError>,
150+
},
151+
Four {
152+
m: HashMap<String, CoverallFlatError>,
153+
},
154+
}
155+
156+
fn output_return_only_dict() -> ReturnOnlyDict {
157+
ReturnOnlyDict {
158+
e: CoverallFlatError::TooManyVariants { num: 1 },
159+
}
160+
}
161+
162+
fn output_return_only_enum() -> ReturnOnlyEnum {
163+
ReturnOnlyEnum::One {
164+
e: CoverallFlatError::TooManyVariants { num: 2 },
165+
}
166+
}
167+
137168
#[derive(Debug, Clone)]
138169
pub struct DictWithDefaults {
139170
name: String,

uniffi_macros/src/error.rs

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ fn flat_error_ffi_converter_impl(
8787
) -> TokenStream {
8888
let name = ident_to_string(ident);
8989
let lower_impl_spec = tagged_impl_header("Lower", ident, udl_mode);
90+
let lift_impl_spec = tagged_impl_header("Lift", ident, udl_mode);
9091
let derive_ffi_traits = derive_ffi_traits(ident, udl_mode, &["ConvertError"]);
9192
let mod_path = match mod_path() {
9293
Ok(p) => p,
@@ -124,11 +125,11 @@ fn flat_error_ffi_converter_impl(
124125
.concat_str(#mod_path)
125126
.concat_str(#name);
126127
}
128+
127129
}
128130
};
129131

130-
let lift_impl = implement_lift.then(|| {
131-
let lift_impl_spec = tagged_impl_header("Lift", ident, udl_mode);
132+
let lift_impl = if implement_lift {
132133
let match_arms = enum_.variants.iter().enumerate().map(|(i, v)| {
133134
let v_ident = &v.ident;
134135
let idx = Index::from(i + 1);
@@ -157,7 +158,31 @@ fn flat_error_ffi_converter_impl(
157158
}
158159

159160
}
160-
});
161+
} else {
162+
quote! {
163+
// Lifting flat errors is not currently supported, but we still define the trait so
164+
// that dicts containing flat errors don't cause compile errors (see ReturnOnlyDict in
165+
// coverall.rs).
166+
//
167+
// Note: it would be better to not derive `Lift` for dictionaries containing flat
168+
// errors, but getting the trait bounds and derived impls there would be much harder.
169+
// For now, we just fail at runtime.
170+
#[automatically_derived]
171+
unsafe #lift_impl_spec {
172+
type FfiType = ::uniffi::RustBuffer;
173+
174+
fn try_read(buf: &mut &[::std::primitive::u8]) -> ::uniffi::deps::anyhow::Result<Self> {
175+
panic!("Can't lift flat errors")
176+
}
177+
178+
fn try_lift(v: ::uniffi::RustBuffer) -> ::uniffi::deps::anyhow::Result<Self> {
179+
panic!("Can't lift flat errors")
180+
}
181+
182+
const TYPE_ID_META: ::uniffi::MetadataBuffer = <Self as ::uniffi::Lower<crate::UniFfiTag>>::TYPE_ID_META;
183+
}
184+
}
185+
};
161186

162187
quote! {
163188
#lower_impl

0 commit comments

Comments
 (0)