Skip to content

Commit fdb2127

Browse files
TartanLlamaTartanLlama
authored
[cpp] Use value types for asymmetric API (#1321)
* Use value types for asymmetric API * Correct test arguments * Update docs * Format * Review feedback * Fix incorrect emoji
1 parent 9c8f2fb commit fdb2127

File tree

7 files changed

+129
-96
lines changed

7 files changed

+129
-96
lines changed

‎crates/cpp/DESIGN.md‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@
1818

1919
| API | | | ABI | |
2020
| --- | --- | --- | --- | --- |
21-
| 🕸 | old | | 📘 | canonical |
22-
| 💎 | new | | 🪞 | symmetric |
21+
| 🌓 | asymmetric | | 📘 | canonical |
22+
| ⚖️ | symmetric | | 🪞 | symmetric |
2323

2424
| Code | mode | WIT Type | Rust type | C++ Type | Lower | Reason |
2525
| --- | --- | --- | --- | --- | --- | --- |
@@ -35,20 +35,20 @@
3535
| GIR | t | string | String | wit::string[^2] | &(addr, len) [^8] | |
3636
| | | list | Vec | wit::vector | &(a,l) |
3737
| | | result<string,list> | Result<String, Vec> | std::expected<wit::string, wit::vector> | &(d,a,l) |
38-
| GEA | t | string | String | 🕸 wit::string&& | addr, len |
39-
| | | | | 💎 string_view | |
40-
| | | result<string,list> | Result<String, Vec> | 🕸 std::expected<wit::string, wit::vector>&& | d,a,l |
41-
| | | | | 💎 std::expected<string_view, wit::span> | |
38+
| GEA | t | string | String | 🌓 wit::string | addr, len |
39+
| | | | | ⚖️ string_view | |
40+
| | | result<string,list> | Result<String, Vec> | 🌓 std::expected<wit::string, wit::vector> | d,a,l |
41+
| | | | | ⚖️ std::expected<string_view, wit::span> | |
4242
| GER | p | string | String | wit::string (or std?) | 📘 -> &(a,l) cabi_post_N:P/I#F [^7] |
4343
| | | | | | 🪞 &(a,l) |
4444
| | | result<string,list> | Result<String, Vec> | std::expected<wit::string, wit::vector> | 📘 -> &(d,a,l) cabi_post |
4545
| --S | ? | string | String | wit::string | addr, len |
4646
| HIA | v | string | | string_view | a,l |
4747
| HIR | t | string | | wit::string[^3] | &(a,l) |
48-
| HEA | t | string | | 🕸 wit::string[^4] | a,l |
49-
| | | | | 💎 string_view [^6] | |
50-
| HER | p | string | | 🕸 wit::guest_owned<string_view> | 📘 -> &(a,l) |
51-
| | | | | 💎 wit::string [^6] | 🪞 &(a,l) |
48+
| HEA | t | string | | 🌓 wit::string[^4] | a,l |
49+
| | | | | ⚖️ string_view [^6] | |
50+
| HER | p | string | | 🌓 wit::guest_owned<string_view> | 📘 -> &(a,l) |
51+
| | | | | ⚖️ wit::string [^6] | 🪞 &(a,l) |
5252

5353
[^1]: The host never frees memory (is never passed ownership)!
5454

‎crates/cpp/src/lib.rs‎

Lines changed: 76 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
1+
use anyhow::bail;
12
use heck::{ToPascalCase, ToShoutySnakeCase, ToSnakeCase, ToUpperCamelCase};
23
use std::{
34
collections::{HashMap, HashSet},
4-
fmt::Write as FmtWrite,
5+
fmt::{Display, Write as FmtWrite},
56
io::{Read, Write},
67
path::PathBuf,
78
process::{Command, Stdio},
@@ -209,32 +210,60 @@ pub struct Opts {
209210
#[cfg_attr(feature = "clap", arg(long, default_value_t = Ownership::Owning))]
210211
pub ownership: Ownership,
211212

212-
/// Symmetric API, same API for imported and exported functions.
213-
/// Reduces the allocation overhead for symmetric ABI.
213+
/// Set API style to symmetric or asymmetric
214214
#[cfg_attr(
215-
feature = "clap",
216-
arg(long, default_value_t = true, overrides_with = "_old_api")
215+
feature = "clap",
216+
arg(
217+
long,
218+
default_value_t = APIStyle::default(),
219+
value_name = "STYLE",
220+
),
217221
)]
218-
pub new_api: bool,
219-
220-
/// Asymmetric API: Imported functions borrow arguments (const&),
221-
/// while exported functions received owned arguments (&&).
222-
/// Reduces the allocation overhead for canonical ABI.
223-
#[cfg_attr(feature = "clap", arg(long))]
224-
pub _old_api: bool,
222+
pub api_style: APIStyle,
225223

226224
/// Where to place output files
227225
#[cfg_attr(feature = "clap", arg(skip))]
228226
out_dir: Option<PathBuf>,
229227
}
230228

229+
/// Supported API styles for the generated bindings.
230+
#[derive(Default, Debug, Copy, Clone, PartialEq, Eq, Hash)]
231+
pub enum APIStyle {
232+
/// Imported functions borrow arguments, while exported functions receive owned arguments. Reduces the allocation overhead for the canonical ABI.
233+
#[default]
234+
Asymmetric,
235+
/// Same API for imported and exported functions. Reduces the allocation overhead for symmetric ABI.
236+
Symmetric,
237+
}
238+
239+
impl Display for APIStyle {
240+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
241+
match self {
242+
APIStyle::Asymmetric => write!(f, "asymmetric"),
243+
APIStyle::Symmetric => write!(f, "symmetric"),
244+
}
245+
}
246+
}
247+
248+
impl FromStr for APIStyle {
249+
type Err = anyhow::Error;
250+
251+
fn from_str(s: &str) -> Result<Self, Self::Err> {
252+
match s {
253+
"asymmetric" => Ok(APIStyle::Asymmetric),
254+
"symmetric" => Ok(APIStyle::Symmetric),
255+
_ => bail!(
256+
"unrecognized API style: `{}`; expected `asymmetric` or `symmetric`",
257+
s
258+
),
259+
}
260+
}
261+
}
262+
231263
impl Opts {
232264
pub fn build(mut self, out_dir: Option<&PathBuf>) -> Box<dyn WorldGenerator> {
233265
let mut r = Cpp::new();
234266
self.out_dir = out_dir.cloned();
235-
if self._old_api {
236-
self.new_api = false;
237-
}
238267
r.opts = self;
239268
Box::new(r)
240269
}
@@ -684,6 +713,7 @@ impl WorldGenerator for Cpp {
684713
.unwrap()
685714
.as_slice(),
686715
);
716+
687717
Ok(())
688718
}
689719
}
@@ -1183,17 +1213,18 @@ impl CppInterfaceGenerator<'_> {
11831213
let special = is_special_method(func);
11841214
if !matches!(special, SpecialMethod::Allocate) {
11851215
self.gen.c_src.src.push_str("{\n");
1186-
let needs_dealloc =
1187-
if self.gen.opts.new_api && matches!(variant, AbiVariant::GuestExport) {
1188-
self.gen
1189-
.c_src
1190-
.src
1191-
.push_str("std::vector<void*> _deallocate;\n");
1192-
self.gen.dependencies.needs_vector = true;
1193-
true
1194-
} else {
1195-
false
1196-
};
1216+
let needs_dealloc = if self.gen.opts.api_style == APIStyle::Symmetric
1217+
&& matches!(variant, AbiVariant::GuestExport)
1218+
{
1219+
self.gen
1220+
.c_src
1221+
.src
1222+
.push_str("std::vector<void*> _deallocate;\n");
1223+
self.gen.dependencies.needs_vector = true;
1224+
true
1225+
} else {
1226+
false
1227+
};
11971228
let lift_lower = if export {
11981229
LiftLower::LiftArgsLowerResults
11991230
} else {
@@ -1402,14 +1433,15 @@ impl CppInterfaceGenerator<'_> {
14021433
"std::string_view".into()
14031434
}
14041435
Flavor::Argument(var)
1405-
if matches!(var, AbiVariant::GuestImport) || self.gen.opts.new_api =>
1436+
if matches!(var, AbiVariant::GuestImport)
1437+
|| self.gen.opts.api_style == APIStyle::Symmetric =>
14061438
{
14071439
self.gen.dependencies.needs_string_view = true;
14081440
"std::string_view".into()
14091441
}
14101442
Flavor::Argument(AbiVariant::GuestExport) => {
14111443
self.gen.dependencies.needs_wit = true;
1412-
"wit::string &&".into()
1444+
"wit::string".into()
14131445
}
14141446
_ => {
14151447
self.gen.dependencies.needs_wit = true;
@@ -1491,14 +1523,15 @@ impl CppInterfaceGenerator<'_> {
14911523
format!("wit::span<{inner} const>")
14921524
}
14931525
Flavor::Argument(var)
1494-
if matches!(var, AbiVariant::GuestImport) || self.gen.opts.new_api =>
1526+
if matches!(var, AbiVariant::GuestImport)
1527+
|| self.gen.opts.api_style == APIStyle::Symmetric =>
14951528
{
14961529
self.gen.dependencies.needs_wit = true;
14971530
format!("wit::span<{inner} const>")
14981531
}
14991532
Flavor::Argument(AbiVariant::GuestExport) => {
15001533
self.gen.dependencies.needs_wit = true;
1501-
format!("wit::vector<{inner}>&&")
1534+
format!("wit::vector<{inner}>")
15021535
}
15031536
_ => {
15041537
self.gen.dependencies.needs_wit = true;
@@ -2280,7 +2313,7 @@ impl<'a, 'b> Bindgen for FunctionBindgen<'a, 'b> {
22802313
.gen
22812314
.type_name(element, &self.namespace, Flavor::InStruct);
22822315
self.push_str(&format!("auto {} = {};\n", len, operands[1]));
2283-
let result = if self.gen.gen.opts.new_api
2316+
let result = if self.gen.gen.opts.api_style == APIStyle::Symmetric
22842317
&& matches!(self.variant, AbiVariant::GuestExport)
22852318
{
22862319
format!(
@@ -2296,7 +2329,7 @@ impl<'a, 'b> Bindgen for FunctionBindgen<'a, 'b> {
22962329
let tmp = self.tmp();
22972330
let len = format!("len{}", tmp);
22982331
uwriteln!(self.src, "auto {} = {};\n", len, operands[1]);
2299-
let result = if self.gen.gen.opts.new_api
2332+
let result = if self.gen.gen.opts.api_style == APIStyle::Symmetric
23002333
&& matches!(self.variant, AbiVariant::GuestExport)
23012334
{
23022335
assert!(self.needs_dealloc);
@@ -2316,7 +2349,7 @@ impl<'a, 'b> Bindgen for FunctionBindgen<'a, 'b> {
23162349
let tmp = self.tmp();
23172350
let size = self.gen.sizes.size(element);
23182351
let _align = self.gen.sizes.align(element);
2319-
let flavor = if self.gen.gen.opts.new_api
2352+
let flavor = if self.gen.gen.opts.api_style == APIStyle::Symmetric
23202353
&& matches!(self.variant, AbiVariant::GuestExport)
23212354
{
23222355
Flavor::BorrowedArgument
@@ -2339,7 +2372,9 @@ impl<'a, 'b> Bindgen for FunctionBindgen<'a, 'b> {
23392372
r#"auto {result} = wit::vector<{vtype}>::allocate({len});
23402373
"#,
23412374
));
2342-
if self.gen.gen.opts.new_api && matches!(self.variant, AbiVariant::GuestExport) {
2375+
if self.gen.gen.opts.api_style == APIStyle::Symmetric
2376+
&& matches!(self.variant, AbiVariant::GuestExport)
2377+
{
23432378
assert!(self.needs_dealloc);
23442379
self.push_str(&format!("if ({len}>0) _deallocate.push_back({base});\n"));
23452380
}
@@ -2359,9 +2394,12 @@ impl<'a, 'b> Bindgen for FunctionBindgen<'a, 'b> {
23592394
// inplace construct
23602395
uwriteln!(self.src, "{result}.initialize(i, std::move(e{tmp}));");
23612396
uwriteln!(self.src, "}}");
2362-
if self.gen.gen.opts.new_api && matches!(self.variant, AbiVariant::GuestExport) {
2397+
if self.gen.gen.opts.api_style == APIStyle::Symmetric
2398+
&& matches!(self.variant, AbiVariant::GuestExport)
2399+
{
23632400
results.push(format!("{result}.get_const_view()"));
2364-
if self.gen.gen.opts.new_api && matches!(self.variant, AbiVariant::GuestExport)
2401+
if self.gen.gen.opts.api_style == APIStyle::Symmetric
2402+
&& matches!(self.variant, AbiVariant::GuestExport)
23652403
{
23662404
self.leak_on_insertion.replace(format!(
23672405
"if ({len}>0) _deallocate.push_back((void*){result}.leak());\n"
@@ -2672,9 +2710,7 @@ impl<'a, 'b> Bindgen for FunctionBindgen<'a, 'b> {
26722710
}
26732711

26742712
let op0 = &operands[0];
2675-
let flavor = if self.gen.gen.opts.new_api
2676-
&& matches!(self.variant, AbiVariant::GuestImport)
2677-
{
2713+
let flavor = if matches!(self.variant, AbiVariant::GuestImport) {
26782714
Flavor::BorrowedArgument
26792715
} else {
26802716
Flavor::InStruct
@@ -2697,7 +2733,7 @@ impl<'a, 'b> Bindgen for FunctionBindgen<'a, 'b> {
26972733
let (_none, none_results) = self.blocks.pop().unwrap();
26982734
assert!(none_results.is_empty());
26992735
assert!(some_results.len() == 1);
2700-
let flavor = if self.gen.gen.opts.new_api
2736+
let flavor = if self.gen.gen.opts.api_style == APIStyle::Symmetric
27012737
&& matches!(self.variant, AbiVariant::GuestExport)
27022738
{
27032739
Flavor::BorrowedArgument

‎tests/runtime/lists/test.cpp‎

Lines changed: 26 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,15 @@ static bool equal(std::tuple<R,S> const&a, std::tuple<T,U> const& b) {
4040
return equal(std::get<0>(a), std::get<0>(b)) && equal(std::get<1>(a), std::get<1>(b));
4141
}
4242

43-
void exports::test::lists::to_test::EmptyListParam(wit::span<uint8_t const> a) {
43+
static bool equal(wit::string const& a, std::string_view b) {
44+
return a.get_view() == b;
45+
}
46+
47+
void exports::test::lists::to_test::EmptyListParam(wit::vector<uint8_t> a) {
4448
assert(a.empty());
4549
}
4650

47-
void exports::test::lists::to_test::EmptyStringParam(std::string_view a) {
51+
void exports::test::lists::to_test::EmptyStringParam(wit::string a) {
4852
assert(a.empty());
4953
}
5054

@@ -56,33 +60,33 @@ wit::string exports::test::lists::to_test::EmptyStringResult() {
5660
return wit::string::from_view(std::string_view());
5761
}
5862

59-
void exports::test::lists::to_test::ListParam(wit::span<const uint8_t> list) {
63+
void exports::test::lists::to_test::ListParam(wit::vector<uint8_t> list) {
6064
assert(equal(list, std::vector<uint8_t>{1, 2, 3, 4}));
6165
}
6266

63-
void exports::test::lists::to_test::ListParam2(std::string_view ptr) {
67+
void exports::test::lists::to_test::ListParam2(wit::string ptr) {
6468
assert(equal(ptr, std::string_view("foo")));
6569
}
6670

67-
void exports::test::lists::to_test::ListParam3(wit::span<const std::string_view> ptr) {
71+
void exports::test::lists::to_test::ListParam3(wit::vector<wit::string> ptr) {
6872
assert(equal(ptr.size(), size_t(3)));
6973
assert(equal(ptr[0], std::string_view("foo")));
7074
assert(equal(ptr[1], std::string_view("bar")));
7175
assert(equal(ptr[2], std::string_view("baz")));
7276
}
7377

74-
void exports::test::lists::to_test::ListParam4(wit::span<const wit::span<const std::string_view>> ptr) {
78+
void exports::test::lists::to_test::ListParam4(wit::vector<wit::vector<wit::string>> ptr) {
7579
assert(equal(ptr.size(), size_t(2)));
7680
assert(equal(ptr[0][0], std::string_view("foo")));
7781
assert(equal(ptr[0][1], std::string_view("bar")));
7882
assert(equal(ptr[1][0], std::string_view("baz")));
7983
}
8084

81-
void exports::test::lists::to_test::ListParam5(wit::span<std::tuple<uint8_t, uint32_t, uint8_t> const> a) {
85+
void exports::test::lists::to_test::ListParam5(wit::vector<std::tuple<uint8_t, uint32_t, uint8_t>> a) {
8286

8387
}
8488

85-
void exports::test::lists::to_test::ListParamLarge(wit::span<std::string_view const> a) {
89+
void exports::test::lists::to_test::ListParamLarge(wit::vector<wit::string> a) {
8690

8791
}
8892

@@ -98,30 +102,30 @@ wit::vector<wit::string> exports::test::lists::to_test::ListResult3() {
98102
return wit::vector<wit::string>::from_view(wit::span<wit::string>(std::vector<wit::string>{wit::string::from_view("hello,"), wit::string::from_view("world!")}));
99103
}
100104

101-
wit::vector<uint8_t> exports::test::lists::to_test::ListRoundtrip(wit::span<const uint8_t> x) {
102-
return wit::vector<uint8_t>::from_view(x);
105+
wit::vector<uint8_t> exports::test::lists::to_test::ListRoundtrip(wit::vector<uint8_t> x) {
106+
return x;
103107
}
104108

105-
wit::string exports::test::lists::to_test::StringRoundtrip(std::string_view x) {
106-
return wit::string::from_view(x);
109+
wit::string exports::test::lists::to_test::StringRoundtrip(wit::string x) {
110+
return x;
107111
}
108112

109-
std::tuple<wit::vector<uint8_t>, wit::vector<int8_t>> exports::test::lists::to_test::ListMinmax8(wit::span<uint8_t const> a, wit::span<int8_t const> b) {
110-
return std::make_tuple(wit::vector<uint8_t>::from_view(a), wit::vector<int8_t>::from_view(b));
113+
std::tuple<wit::vector<uint8_t>, wit::vector<int8_t>> exports::test::lists::to_test::ListMinmax8(wit::vector<uint8_t> a, wit::vector<int8_t> b) {
114+
return std::make_tuple(std::move(a), std::move(b));
111115
}
112116

113-
std::tuple<wit::vector<uint16_t>, wit::vector<int16_t>> exports::test::lists::to_test::ListMinmax16(wit::span<uint16_t const> a, wit::span<int16_t const> b) {
114-
return std::make_tuple(wit::vector<uint16_t>::from_view(a), wit::vector<int16_t>::from_view(b));
117+
std::tuple<wit::vector<uint16_t>, wit::vector<int16_t>> exports::test::lists::to_test::ListMinmax16(wit::vector<uint16_t> a, wit::vector<int16_t> b) {
118+
return std::make_tuple(std::move(a), std::move(b));
115119
}
116120

117-
std::tuple<wit::vector<uint32_t>, wit::vector<int32_t>> exports::test::lists::to_test::ListMinmax32(wit::span<uint32_t const> a, wit::span<int32_t const> b) {
118-
return std::make_tuple(wit::vector<uint32_t>::from_view(a), wit::vector<int32_t>::from_view(b));
121+
std::tuple<wit::vector<uint32_t>, wit::vector<int32_t>> exports::test::lists::to_test::ListMinmax32(wit::vector<uint32_t> a, wit::vector<int32_t> b) {
122+
return std::make_tuple(std::move(a), std::move(b));
119123
}
120124

121-
std::tuple<wit::vector<uint64_t>, wit::vector<int64_t>> exports::test::lists::to_test::ListMinmax64(wit::span<uint64_t const> a, wit::span<int64_t const> b) {
122-
return std::make_tuple(wit::vector<uint64_t>::from_view(a), wit::vector<int64_t>::from_view(b));
125+
std::tuple<wit::vector<uint64_t>, wit::vector<int64_t>> exports::test::lists::to_test::ListMinmax64(wit::vector<uint64_t> a, wit::vector<int64_t> b) {
126+
return std::make_tuple(std::move(a), std::move(b));
123127
}
124128

125-
std::tuple<wit::vector<float>, wit::vector<double>> exports::test::lists::to_test::ListMinmaxFloat(wit::span<float const> a, wit::span<double const> b) {
126-
return std::make_tuple(wit::vector<float>::from_view(a), wit::vector<double>::from_view(b));
129+
std::tuple<wit::vector<float>, wit::vector<double>> exports::test::lists::to_test::ListMinmaxFloat(wit::vector<float> a, wit::vector<double> b) {
130+
return std::make_tuple(std::move(a), std::move(b));
127131
}

0 commit comments

Comments
 (0)