Skip to content

Commit

Permalink
C#: use nullable types for non-nested options (#977)
Browse files Browse the repository at this point in the history 
We now generate bindings which use nullable types (e.g. `uint?`) to represent
non-nested `option` types.  Nested `option`s, such as `option<option<T>>` still
use the `Option` class except for the innermost `option` in order to avoid
ambiguity.

Fixes #964

Signed-off-by: Joel Dice <joel.dice@fermyon.com>
  • Loading branch information
@dicej
dicej committed Jun 19, 2024
1 parent 86d568a commit 4336237
Show file tree
Hide file tree
Showing 6 changed files with 135 additions and 25 deletions.
63 changes: 46 additions & 17 deletions crates/csharp/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1298,16 +1298,16 @@ impl InterfaceGenerator<'_> {
}
TypeDefKind::Option(base_ty) => {
self.gen.needs_option = true;
if let Some(_name) = &ty.name {
format!(
"Option<{}>",
self.type_name_with_qualifier(base_ty, qualifier)
)
let nesting = if let Type::Id(id) = base_ty {
matches!(&self.resolve.types[*id].kind, TypeDefKind::Option(_))
} else {
format!(
"Option<{}>",
self.type_name_with_qualifier(base_ty, qualifier)
)
false
};
let base_ty = self.type_name_with_qualifier(base_ty, qualifier);
if nesting {
format!("Option<{base_ty}>")
} else {
format!("{base_ty}?")
}
}
TypeDefKind::Result(result) => {
Expand Down Expand Up @@ -2300,9 +2300,20 @@ impl Bindgen for FunctionBindgen<'_, '_> {

let op = &operands[0];

let block = |ty: Option<&Type>, Block { body, results, .. }, payload| {
let payload = if let Some(_ty) = self.gen.non_empty_type(ty) {
format!("var {payload} = {op}.Value;")
let nesting = if let Type::Id(id) = payload {
matches!(&self.gen.resolve.types[*id].kind, TypeDefKind::Option(_))
} else {
false
};

let mut block = |ty: Option<&Type>, Block { body, results, .. }, payload, nesting| {
let payload = if let Some(ty) = self.gen.non_empty_type(ty) {
let ty = self.gen.type_name_with_qualifier(ty, true);
if nesting {
format!("var {payload} = {op}.Value;")
} else {
format!("var {payload} = ({ty}) {op};")
}
} else {
String::new()
};
Expand All @@ -2321,15 +2332,21 @@ impl Bindgen for FunctionBindgen<'_, '_> {
)
};

let none = block(None, none, none_payload);
let some = block(Some(payload), some, some_payload);
let none = block(None, none, none_payload, nesting);
let some = block(Some(payload), some, some_payload, nesting);

let test = if nesting {
".HasValue"
} else {
" != null"
};

uwrite!(
self.src,
r#"
{declarations}
if ({op}.HasValue) {{
if ({op}{test}) {{
{some}
}} else {{
{none}
Expand All @@ -2346,6 +2363,12 @@ impl Bindgen for FunctionBindgen<'_, '_> {
let lifted = self.locals.tmp("lifted");
let op = &operands[0];

let nesting = if let Type::Id(id) = payload {
matches!(&self.gen.resolve.types[*id].kind, TypeDefKind::Option(_))
} else {
false
};

let payload = if self.gen.non_empty_type(Some(*payload)).is_some() {
some.results.into_iter().next().unwrap()
} else {
Expand All @@ -2354,20 +2377,26 @@ impl Bindgen for FunctionBindgen<'_, '_> {

let some = some.body;

let (none_value, some_value) = if nesting {
(format!("{ty}.None"), format!("new ({payload})"))
} else {
("null".into(), payload)
};

uwrite!(
self.src,
r#"
{ty} {lifted};
switch ({op}) {{
case 0: {{
{lifted} = {ty}.None;
{lifted} = {none_value};
break;
}}
case 1: {{
{some}
{lifted} = new ({payload});
{lifted} = {some_value};
break;
}}
Expand Down
16 changes: 16 additions & 0 deletions tests/runtime/options.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ impl test::options::test::Host for MyImports {
fn option_roundtrip(&mut self, a: Option<String>) -> Result<Option<String>> {
Ok(a)
}

fn double_option_roundtrip(&mut self, a: Option<Option<u32>>) -> Result<Option<Option<u32>>> {
Ok(a)
}
}

#[test]
Expand All @@ -54,5 +58,17 @@ fn run_test(exports: Options, store: &mut Store<crate::Wasi<MyImports>>) -> Resu
exports.call_option_roundtrip(&mut *store, Some("foo"))?,
Some("foo".to_string())
);
assert_eq!(
exports.call_double_option_roundtrip(&mut *store, Some(Some(42)))?,
Some(Some(42))
);
assert_eq!(
exports.call_double_option_roundtrip(&mut *store, Some(None))?,
Some(None)
);
assert_eq!(
exports.call_double_option_roundtrip(&mut *store, None)?,
None
);
Ok(())
}
56 changes: 56 additions & 0 deletions tests/runtime/options/wasm.cs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
using System.Diagnostics;
using OptionsWorld.wit.imports.test.options;

namespace OptionsWorld.wit.exports.test.options
{
public class TestImpl : ITest
{
public static void OptionNoneParam(string? a)
{
Debug.Assert(a == null);
}

public static string? OptionNoneResult()
{
return null;
}

public static void OptionSomeParam(string? a)
{
Debug.Assert(a == "foo");
}

public static string? OptionSomeResult()
{
return "foo";
}

public static string? OptionRoundtrip(string? a)
{
return a;
}

public static Option<uint?> DoubleOptionRoundtrip(Option<uint?> a)
{
return a;
}
}
}

namespace OptionsWorld
{
public class OptionsWorldImpl : IOptionsWorld
{
public static void TestImports()
{
TestInterop.OptionNoneParam(null);
TestInterop.OptionSomeParam("foo");
Debug.Assert(TestInterop.OptionNoneResult() == null);
Debug.Assert(TestInterop.OptionSomeResult() == "foo");
Debug.Assert(TestInterop.OptionRoundtrip("foo") == "foo");
Debug.Assert(TestInterop.DoubleOptionRoundtrip(new Option<uint?>(42)).Value == 42);
Debug.Assert(TestInterop.DoubleOptionRoundtrip(new Option<uint?>(null)).Value == null);
Debug.Assert(!TestInterop.DoubleOptionRoundtrip(Option<uint?>.None).HasValue);
}
}
}
7 changes: 7 additions & 0 deletions tests/runtime/options/wasm.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ impl Guest for Component {
assert!(option_none_result().is_none());
assert_eq!(option_some_result(), Some("foo".to_string()));
assert_eq!(option_roundtrip(Some("foo")), Some("foo".to_string()));
assert_eq!(double_option_roundtrip(Some(Some(42))), Some(Some(42)));
assert_eq!(double_option_roundtrip(Some(None)), Some(None));
assert_eq!(double_option_roundtrip(None), None);
}
}

Expand All @@ -38,4 +41,8 @@ impl exports::test::options::test::Guest for Component {
fn option_roundtrip(a: Option<String>) -> Option<String> {
a
}

fn double_option_roundtrip(a: Option<Option<u32>>) -> Option<Option<u32>> {
a
}
}
2 changes: 2 additions & 0 deletions tests/runtime/options/world.wit
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ interface test {
option-some-result: func() -> option<string>;

option-roundtrip: func(a: option<string>) -> option<string>;

double-option-roundtrip: func(a: option<option<u32>>) -> option<option<u32>>;
}

world options {
Expand Down
16 changes: 8 additions & 8 deletions tests/runtime/resource_aggregates/wasm.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ public class Thing : ITest.Thing, ITest.IThing {
ITest.V2 v2,
List<ITest.Thing> l1,
List<ITest.Thing> l2,
Option<ITest.Thing> o1,
Option<ITest.Thing> o2,
ITest.Thing? o1,
ITest.Thing? o2,
Result<ITest.Thing, None> result1,
Result<ITest.Thing, None> result2
)
Expand All @@ -45,12 +45,12 @@ public class Thing : ITest.Thing, ITest.IThing {
{
il2.Add(((Thing) thing).val);
}
var io1 = o1.HasValue
? new Option<Import.Thing>(((Thing) o1.Value).val)
: Option<Import.Thing>.None;
var io2 = o2.HasValue
? new Option<Import.Thing>(((Thing) o2.Value).val)
: Option<Import.Thing>.None;
var io1 = o1 != null
? ((Thing) o1).val
: null;
var io2 = o2 != null
? ((Thing) o2).val
: null;
var iresult1 = result1.IsOk
? Result<Import.Thing, None>.ok(((Thing) result1.AsOk).val)
: Result<Import.Thing, None>.err(new None());
Expand Down

0 comments on commit 4336237

Please sign in to comment.