Skip to content

C#: Use fully qualified type names #1275

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 13, 2025
Merged

C#: Use fully qualified type names #1275

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/csharp/src/RepTable.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ internal int Add(T v) {

internal T Get(nint rep) {
if (list[(int)rep] is Vacant) {
throw new ArgumentException("invalid rep");
throw new global::System.ArgumentException("invalid rep");
}
return (T) list[(int)rep];
}
Expand Down
97 changes: 42 additions & 55 deletions crates/csharp/src/function.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ impl<'a, 'b> FunctionBindgen<'a, 'b> {
switch ({op}.Tag) {{
{cases}

default: throw new ArgumentException($"invalid discriminant: {{{op}}}");
default: throw new global::System.ArgumentException("invalid discriminant: " + {op});
}}
"#
);
Expand Down Expand Up @@ -222,7 +222,7 @@ impl<'a, 'b> FunctionBindgen<'a, 'b> {
switch ({op}) {{
{cases}

default: throw new ArgumentException($"invalid discriminant: {{{op}}}");
default: throw new global::System.ArgumentException("invalid discriminant:" + {op});
}}
"#
);
Expand Down Expand Up @@ -363,7 +363,7 @@ impl<'a, 'b> FunctionBindgen<'a, 'b> {
{{
{cases}

default: throw new ArgumentException($"invalid nesting level: {{e.NestingLevel}}");
default: throw new global::System.ArgumentException($"invalid nesting level: {{e.NestingLevel}}");
}}
}}
"#
Expand Down Expand Up @@ -400,22 +400,22 @@ impl Bindgen for FunctionBindgen<'_, '_> {
})),
Instruction::I32Load { offset }
| Instruction::PointerLoad { offset }
| Instruction::LengthLoad { offset } => results.push(format!("BitConverter.ToInt32(new Span<byte>((void*)({} + {offset}), 4))",operands[0],offset = offset.size_wasm32())),
Instruction::I32Load8U { offset } => results.push(format!("new Span<byte>((void*)({} + {offset}), 1)[0]",operands[0],offset = offset.size_wasm32())),
Instruction::I32Load8S { offset } => results.push(format!("(sbyte)new Span<byte>((void*)({} + {offset}), 1)[0]",operands[0],offset = offset.size_wasm32())),
Instruction::I32Load16U { offset } => results.push(format!("BitConverter.ToUInt16(new Span<byte>((void*)({} + {offset}), 2))",operands[0],offset = offset.size_wasm32())),
Instruction::I32Load16S { offset } => results.push(format!("BitConverter.ToInt16(new Span<byte>((void*)({} + {offset}), 2))",operands[0],offset = offset.size_wasm32())),
Instruction::I64Load { offset } => results.push(format!("BitConverter.ToInt64(new Span<byte>((void*)({} + {offset}), 8))",operands[0],offset = offset.size_wasm32())),
Instruction::F32Load { offset } => results.push(format!("BitConverter.ToSingle(new Span<byte>((void*)({} + {offset}), 4))",operands[0],offset = offset.size_wasm32())),
Instruction::F64Load { offset } => results.push(format!("BitConverter.ToDouble(new Span<byte>((void*)({} + {offset}), 8))",operands[0],offset = offset.size_wasm32())),
| Instruction::LengthLoad { offset } => results.push(format!("global::System.BitConverter.ToInt32(new global::System.Span<byte>((void*)({} + {offset}), 4))",operands[0],offset = offset.size_wasm32())),
Instruction::I32Load8U { offset } => results.push(format!("new global::System.Span<byte>((void*)({} + {offset}), 1)[0]",operands[0],offset = offset.size_wasm32())),
Instruction::I32Load8S { offset } => results.push(format!("(sbyte)new global::System.Span<byte>((void*)({} + {offset}), 1)[0]",operands[0],offset = offset.size_wasm32())),
Instruction::I32Load16U { offset } => results.push(format!("global::System.BitConverter.ToUInt16(new global::System.Span<byte>((void*)({} + {offset}), 2))",operands[0],offset = offset.size_wasm32())),
Instruction::I32Load16S { offset } => results.push(format!("global::System.BitConverter.ToInt16(new global::System.Span<byte>((void*)({} + {offset}), 2))",operands[0],offset = offset.size_wasm32())),
Instruction::I64Load { offset } => results.push(format!("global::System.BitConverter.ToInt64(new global::System.Span<byte>((void*)({} + {offset}), 8))",operands[0],offset = offset.size_wasm32())),
Instruction::F32Load { offset } => results.push(format!("global::System.BitConverter.ToSingle(new global::System.Span<byte>((void*)({} + {offset}), 4))",operands[0],offset = offset.size_wasm32())),
Instruction::F64Load { offset } => results.push(format!("global::System.BitConverter.ToDouble(new global::System.Span<byte>((void*)({} + {offset}), 8))",operands[0],offset = offset.size_wasm32())),
Instruction::I32Store { offset }
| Instruction::PointerStore { offset }
| Instruction::LengthStore { offset } => uwriteln!(self.src, "BitConverter.TryWriteBytes(new Span<byte>((void*)({} + {offset}), 4), {});", operands[1], operands[0],offset = offset.size_wasm32()),
| Instruction::LengthStore { offset } => uwriteln!(self.src, "global::System.BitConverter.TryWriteBytes(new global::System.Span<byte>((void*)({} + {offset}), 4), {});", operands[1], operands[0],offset = offset.size_wasm32()),
Instruction::I32Store8 { offset } => uwriteln!(self.src, "*(byte*)({} + {offset}) = (byte){};", operands[1], operands[0],offset = offset.size_wasm32()),
Instruction::I32Store16 { offset } => uwriteln!(self.src, "BitConverter.TryWriteBytes(new Span<byte>((void*)({} + {offset}), 2), (short){});", operands[1], operands[0],offset = offset.size_wasm32()),
Instruction::I64Store { offset } => uwriteln!(self.src, "BitConverter.TryWriteBytes(new Span<byte>((void*)({} + {offset}), 8), unchecked((long){}));", operands[1], operands[0],offset = offset.size_wasm32()),
Instruction::F32Store { offset } => uwriteln!(self.src, "BitConverter.TryWriteBytes(new Span<byte>((void*)({} + {offset}), 4), unchecked((float){}));", operands[1], operands[0],offset = offset.size_wasm32()),
Instruction::F64Store { offset } => uwriteln!(self.src, "BitConverter.TryWriteBytes(new Span<byte>((void*)({} + {offset}), 8), unchecked((double){}));", operands[1], operands[0],offset = offset.size_wasm32()),
Instruction::I32Store16 { offset } => uwriteln!(self.src, "global::System.BitConverter.TryWriteBytes(new global::System.Span<byte>((void*)({} + {offset}), 2), (short){});", operands[1], operands[0],offset = offset.size_wasm32()),
Instruction::I64Store { offset } => uwriteln!(self.src, "global::System.BitConverter.TryWriteBytes(new global::System.Span<byte>((void*)({} + {offset}), 8), unchecked((long){}));", operands[1], operands[0],offset = offset.size_wasm32()),
Instruction::F32Store { offset } => uwriteln!(self.src, "global::System.BitConverter.TryWriteBytes(new global::System.Span<byte>((void*)({} + {offset}), 4), unchecked((float){}));", operands[1], operands[0],offset = offset.size_wasm32()),
Instruction::F64Store { offset } => uwriteln!(self.src, "global::System.BitConverter.TryWriteBytes(new global::System.Span<byte>((void*)({} + {offset}), 8), unchecked((double){}));", operands[1], operands[0],offset = offset.size_wasm32()),

Instruction::I64FromU64 => results.push(format!("unchecked((long)({}))", operands[0])),
Instruction::I32FromChar => results.push(format!("((int){})", operands[0])),
Expand Down Expand Up @@ -680,7 +680,7 @@ impl Bindgen for FunctionBindgen<'_, '_> {
break;
}}

default: throw new ArgumentException("invalid discriminant: " + ({op}));
default: throw new global::System.ArgumentException("invalid discriminant: " + ({op}));
}}
"#
);
Expand Down Expand Up @@ -748,7 +748,7 @@ impl Bindgen for FunctionBindgen<'_, '_> {
uwrite!(
self.src,
"
var {handle} = GCHandle.Alloc({list}, GCHandleType.Pinned);
var {handle} = global::System.Runtime.InteropServices.GCHandle.Alloc({list}, global::System.Runtime.InteropServices.GCHandleType.Pinned);
var {ptr} = {handle}.AddrOfPinnedObject();
cleanups.Add(()=> {handle}.Free());
"
Expand All @@ -766,8 +766,8 @@ impl Bindgen for FunctionBindgen<'_, '_> {
self.src,
"
var {byte_length} = ({size}) * {list}.Length;
var {address} = NativeMemory.Alloc((nuint)({byte_length}));
{list}.AsSpan().CopyTo(new Span<{ty}>({address},{byte_length}));
var {address} = global::System.Runtime.InteropServices.NativeMemory.Alloc((nuint)({byte_length}));
global::System.MemoryExtensions.AsSpan({list}).CopyTo(new global::System.Span<{ty}>({address},{byte_length}));
"
);

Expand All @@ -787,7 +787,7 @@ impl Bindgen for FunctionBindgen<'_, '_> {
self.src,
"
var {array} = new {ty}[{length}];
new Span<{ty}>((void*)({address}), {length}).CopyTo(new Span<{ty}>({array}));
new global::System.Span<{ty}>((void*)({address}), {length}).CopyTo(new global::System.Span<{ty}>({array}));
"
);

Expand All @@ -805,9 +805,9 @@ impl Bindgen for FunctionBindgen<'_, '_> {
uwriteln!(
self.src,
"
var {utf8_bytes} = Encoding.UTF8.GetBytes({op});
var {utf8_bytes} = global::System.Text.Encoding.UTF8.GetBytes({op});
var {length} = {utf8_bytes}.Length;
var {gc_handle} = GCHandle.Alloc({utf8_bytes}, GCHandleType.Pinned);
var {gc_handle} = global::System.Runtime.InteropServices.GCHandle.Alloc({utf8_bytes}, global::System.Runtime.InteropServices.GCHandleType.Pinned);
var {str_ptr} = {gc_handle}.AddrOfPinnedObject();
"
);
Expand All @@ -825,34 +825,21 @@ impl Bindgen for FunctionBindgen<'_, '_> {
uwriteln!(
self.src,
"
var {string_span} = {op}.AsSpan();
var {length} = Encoding.UTF8.GetByteCount({string_span});
var {str_ptr} = NativeMemory.Alloc((nuint){length});
Encoding.UTF8.GetBytes({string_span}, new Span<byte>({str_ptr}, {length}));
var {string_span} = global::System.MemoryExtensions.AsSpan({op});
var {length} = global::System.Text.Encoding.UTF8.GetByteCount({string_span});
var {str_ptr} = global::System.Runtime.InteropServices.NativeMemory.Alloc((nuint){length});
global::System.Text.Encoding.UTF8.GetBytes({string_span}, new global::System.Span<byte>({str_ptr}, {length}));
"
);
results.push(format!("(int){str_ptr}"));
}

results.push(format!("{length}"));
if FunctionKind::Freestanding == *self.kind || self.interface_gen.direction == Direction::Export {
self.interface_gen.require_interop_using("System.Text");
self.interface_gen.require_interop_using("System.Runtime.InteropServices");
} else {
self.interface_gen.require_using("System.Text");
self.interface_gen.require_using("System.Runtime.InteropServices");
}
}

Instruction::StringLift { .. } => {
if FunctionKind::Freestanding == *self.kind || self.interface_gen.direction == Direction::Export {
self.interface_gen.require_interop_using("System.Text");
} else {
self.interface_gen.require_using("System.Text");
}

results.push(format!(
"Encoding.UTF8.GetString((byte*){}, {})",
"global::System.Text.Encoding.UTF8.GetString((byte*){}, {})",
operands[0], operands[1]
));
}
Expand Down Expand Up @@ -895,8 +882,8 @@ impl Bindgen for FunctionBindgen<'_, '_> {
else
{{
var {buffer_size} = {size} * (nuint){list}.Count;
{address} = NativeMemory.AlignedAlloc({buffer_size}, {align});
cleanups.Add(() => NativeMemory.AlignedFree({address}));
{address} = global::System.Runtime.InteropServices.NativeMemory.AlignedAlloc({buffer_size}, {align});
cleanups.Add(() => global::System.Runtime.InteropServices.NativeMemory.AlignedFree({address}));
}}
"
);
Expand All @@ -906,7 +893,7 @@ impl Bindgen for FunctionBindgen<'_, '_> {
uwrite!(self.src,
"
var {buffer_size} = {size} * (nuint){list}.Count;
void* {address} = NativeMemory.AlignedAlloc({buffer_size}, {align});
void* {address} = global::System.Runtime.InteropServices.NativeMemory.AlignedAlloc({buffer_size}, {align});
"
);
}
Expand Down Expand Up @@ -948,7 +935,7 @@ impl Bindgen for FunctionBindgen<'_, '_> {
uwrite!(
self.src,
"
var {array} = new List<{ty}>({length});
var {array} = new global::System.Collections.Generic.List<{ty}>({length});
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does type_name_with_qualifier give ty the global name too ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not believe so. Please lead me in the right direction to make this happen. It should be unnecessary as long as ty is either a primitive or a type already contained within the same namespace as this emitted code.

Copy link
Collaborator

@pavelsavara pavelsavara Apr 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know much about the generator codebase, @jsturtevant please advise.

I just noticed that different flavors ty is sprinkled thru the code.

I'm thinking that we could have built-in hash table (or switch) translating known system types from short name to long name. All of them for consistency sake.

For int -> global::System.Int32

I guess later we will also generate marshaling for global::System.Threading.Tasks.Task which is not in System namespace. Also global::System.Threading.Tasks.Task<global::System.Int32>

And I think WIT can do list of list ?
global::System.Collections.Generic.List<global::System.Collections.Generic.List<global::System.Int32>>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't personally believe that primitives (int) should be fully specified, but that's up to you.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not big deal for me, I mentioned it for consistency. This generated code should not be something that people need to read often and compiler sees that as equivalent.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does type_name_with_qualifier give ty the global name too ?

No type_name_with_qualifier will not add global:: in-front of the type name.

And I think WIT can do list of list ?

Yes it can.

I like that this makes the code generation part a lot easier. On a personal opinion thought I don't particularly like the fact that we will be generating code that you would normally never write yourself (I know this isn't true for everything already, but I would prefer if we can take it in that direction). Would adjusting the current code to address the scenario of usage of types that haven't been imported (by adding the using statement in the generated code, for those missing places) be sufficient or do you think there is a broader problem with that approach @just-ero ?

Copy link
Contributor Author

@just-ero just-ero Apr 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since wit-bindgen is a standalone tool (as in, not directly connected to componentize-dotnet), it seems reasonable to me to generate code that a human might write.

However, a couple things to dissuade;

  • Since the generated header explicitly discourages editing of the generated code, users would interact with it very rarely. The appearance of the generated code would not matter in most cases, as it is rarely even observed.
  • Fully qualified type names are safer, as they leave no ambiguity. Please read string vs. String is not a style debate by Jared Parsons.
    To be clear: in our case, the fully qualified type name would be closer to the string variant, including namespaces and using just the type name is closer to the String variant.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, one requirement of generated code (at least for .NET) is to fulfill the lowest common denominator. In C# that might mean not using interpolated strings, file-scoped namespaces, or other more modern features, as the user may target a language version which does not support those features.

componentize-dotnet is basically hard locked to projects which target .NET 10 and up, so this would only be a problem if the user explicitly downgraded using the LangVersion MSBuild property. It's not a scenario I would worry too much about, but I thought I'd mention it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not worry about below Net10

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fully qualified type names are safer, as they leave no ambiguity.

This is compelling to me for generated code. So I am +1 to this change.

for (int {index} = 0; {index} < {length}; ++{index}) {{
nint {base} = {address} + ({index} * {size});
{body}
Expand Down Expand Up @@ -1054,7 +1041,7 @@ impl Bindgen for FunctionBindgen<'_, '_> {
}

if self.needs_cleanup {
self.src.insert_str(0, "var cleanups = new List<Action>();
self.src.insert_str(0, "var cleanups = new global::System.Collections.Generic.List<global::System.Action>();
");

uwriteln!(self.src, "
Expand Down Expand Up @@ -1086,11 +1073,11 @@ impl Bindgen for FunctionBindgen<'_, '_> {

Instruction::GuestDeallocate { .. } => {
// the original alloc here comes from cabi_realloc implementation (wasi-libc in .net)
uwriteln!(self.src, r#"NativeMemory.Free((void*){});"#, operands[0]);
uwriteln!(self.src, r#"global::System.Runtime.InteropServices.NativeMemory.Free((void*){});"#, operands[0]);
}

Instruction::GuestDeallocateString => {
uwriteln!(self.src, r#"NativeMemory.Free((void*){});"#, operands[0]);
uwriteln!(self.src, r#"global::System.Runtime.InteropServices.NativeMemory.Free((void*){});"#, operands[0]);
}

Instruction::GuestDeallocateVariant { blocks } => {
Expand Down Expand Up @@ -1150,7 +1137,7 @@ impl Bindgen for FunctionBindgen<'_, '_> {
);
}

uwriteln!(self.src, r#"NativeMemory.Free((void*){});"#, operands[0]);
uwriteln!(self.src, r#"global::System.Runtime.InteropServices.NativeMemory.Free((void*){});"#, operands[0]);
}

Instruction::HandleLower {
Expand Down Expand Up @@ -1385,12 +1372,12 @@ fn list_element_info(ty: &Type) -> (usize, &'static str) {

fn perform_cast(op: &String, cast: &Bitcast) -> String {
match cast {
Bitcast::I32ToF32 => format!("BitConverter.Int32BitsToSingle((int){op})"),
Bitcast::I64ToF32 => format!("BitConverter.Int32BitsToSingle((int){op})"),
Bitcast::F32ToI32 => format!("BitConverter.SingleToInt32Bits({op})"),
Bitcast::F32ToI64 => format!("BitConverter.SingleToInt32Bits({op})"),
Bitcast::I64ToF64 => format!("BitConverter.Int64BitsToDouble({op})"),
Bitcast::F64ToI64 => format!("BitConverter.DoubleToInt64Bits({op})"),
Bitcast::I32ToF32 => format!("global::System.BitConverter.Int32BitsToSingle((int){op})"),
Bitcast::I64ToF32 => format!("global::System.BitConverter.Int32BitsToSingle((int){op})"),
Bitcast::F32ToI32 => format!("global::System.BitConverter.SingleToInt32Bits({op})"),
Bitcast::F32ToI64 => format!("global::System.BitConverter.SingleToInt32Bits({op})"),
Bitcast::I64ToF64 => format!("global::System.BitConverter.Int64BitsToDouble({op})"),
Bitcast::F64ToI64 => format!("global::System.BitConverter.DoubleToInt64Bits({op})"),
Bitcast::I32ToI64 => format!("(long) ({op})"),
Bitcast::I64ToI32 => format!("(int) ({op})"),
Bitcast::I64ToP64 => format!("{op}"),
Expand Down
Loading