From ee2c912bd2c4bd05e79b4652de0a650cdaad40b1 Mon Sep 17 00:00:00 2001 From: Alexander Medvedev Date: Fri, 30 Oct 2026 11:18:29 +0100 Subject: [PATCH 1/2] Fix C# and Go identifier and member name collisions Fixes duplicate definitions and member name collisions in C# (and Go) bindings generation (#1731, #1641, #1703, #1515): - Disambiguate variant `Tag` field, `Tags` nested class, `Tags.` constants, `As` payload accessors, and static factory methods when variant names or case names collide. - Disambiguate resource `Handle` property, `THandle` struct, `WasmInterop` nested class, `Dispose` implementation, and resource method/static function names when resource or method names collide. - Qualify `global::System.GC.SuppressFinalize(this)` in exported resources. - Allocate `cleanups`, `cleanup`, and async `result` locals via `Ns` so they do not shadow function parameters. - Convert WIT identifiers to camelCase before checking against C# keywords in `to_csharp_ident` / `to_csharp_ident_upper`. - Repeat `*/` sanitization in `print_docs` to handle nested comment closures like `**/`. - Use the mangled payload getter name (`_()`) when lowering variants in Go if the case name collides with a reserved variant method. --- crates/csharp/Cargo.toml | 1 - crates/csharp/src/csharp_ident.rs | 14 +- crates/csharp/src/function.rs | 234 +++++++++++++++++++--------- crates/csharp/src/interface.rs | 250 ++++++++++++++++++++++-------- crates/csharp/src/lib.rs | 78 ++++++++++ crates/go/src/lib.rs | 6 +- tests/codegen/issue1731.wit | 124 +++++++++++++++ 7 files changed, 556 insertions(+), 151 deletions(-) create mode 100644 tests/codegen/issue1731.wit diff --git a/crates/csharp/Cargo.toml b/crates/csharp/Cargo.toml index 99ab860a5..9d5c2880a 100644 --- a/crates/csharp/Cargo.toml +++ b/crates/csharp/Cargo.toml @@ -16,7 +16,6 @@ workspace = true [lib] doctest = false -test = false [dependencies] wit-bindgen-core = { workspace = true } diff --git a/crates/csharp/src/csharp_ident.rs b/crates/csharp/src/csharp_ident.rs index 9112d18af..6a2cfbf54 100644 --- a/crates/csharp/src/csharp_ident.rs +++ b/crates/csharp/src/csharp_ident.rs @@ -92,20 +92,22 @@ impl ToCSharpIdent for str { } fn to_csharp_ident(&self) -> String { + let ident = self.to_lower_camel_case(); // Escape C# keywords - if Self::csharp_keywords().contains(&self) { - format!("@{self}") + if Self::csharp_keywords().contains(&ident.as_str()) { + format!("@{ident}") } else { - self.to_lower_camel_case() + ident } } fn to_csharp_ident_upper(&self) -> String { + let ident = self.to_upper_camel_case(); // Escape C# keywords - if Self::csharp_keywords().contains(&self) { - format!("@{self}") + if Self::csharp_keywords().contains(&ident.as_str()) { + format!("@{ident}") } else { - self.to_upper_camel_case() + ident } } } diff --git a/crates/csharp/src/function.rs b/crates/csharp/src/function.rs index 76a1c8604..21744d27a 100644 --- a/crates/csharp/src/function.rs +++ b/crates/csharp/src/function.rs @@ -1,5 +1,9 @@ use crate::csharp_ident::ToCSharpIdent; -use crate::interface::{InterfaceGenerator, ParameterType, variant_new_func_name}; +use crate::interface::{ + InterfaceGenerator, ParameterType, resource_func_name, resource_handle_property_name, + resource_thandle_struct_name, resource_wasm_interop_class_name, variant_accessor_name, + variant_new_func_name, variant_tag_field_name, +}; use crate::world_generator::CSharp; use heck::ToUpperCamelCase; use regex::Regex; @@ -22,11 +26,12 @@ pub(crate) struct FunctionBindgen<'a, 'b> { params: Box<[String]>, results: Vec, pub(crate) src: String, - locals: Ns, + pub(crate) locals: Ns, block_storage: Vec, blocks: Vec, payloads: Vec, pub(crate) needs_cleanup: bool, + cleanups: Option, import_return_pointer_area_size: usize, import_return_pointer_area_align: usize, pub(crate) resource_drops: Vec<(String, String)>, @@ -67,6 +72,7 @@ impl<'a, 'b> FunctionBindgen<'a, 'b> { blocks: Vec::new(), payloads: Vec::new(), needs_cleanup: false, + cleanups: None, import_return_pointer_area_size: 0, import_return_pointer_area_align: 0, resource_drops: Vec::new(), @@ -79,6 +85,16 @@ impl<'a, 'b> FunctionBindgen<'a, 'b> { } } + fn cleanups_var(&mut self) -> String { + if let Some(cleanups) = &self.cleanups { + cleanups.clone() + } else { + let cleanups = self.locals.tmp("cleanups"); + self.cleanups = Some(cleanups.clone()); + cleanups + } + } + /// Index of a future or stream type among the function's futures and streams, which the /// canonical ABI numbers together. Payloads nested inside another future or stream are not in /// that list and use 0. @@ -91,6 +107,7 @@ impl<'a, 'b> FunctionBindgen<'a, 'b> { fn lower_variant( &mut self, + variant_name: &str, cases: &[(&str, Option)], lowered_types: &[WasmType], op: &str, @@ -129,9 +146,9 @@ impl<'a, 'b> FunctionBindgen<'a, 'b> { |(i, (((name, ty), Block { body, results, .. }), payload))| { let payload = if let Some(ty) = self.interface_gen.non_empty_type(ty.as_ref()) { let ty = self.interface_gen.type_name_with_qualifier(ty, true); - let name = name.to_upper_camel_case(); + let accessor_name = variant_accessor_name(variant_name, name); - format!("{ty} {payload} = {op}.As{name};") + format!("{ty} {payload} = {op}.{accessor_name};") } else { String::new() }; @@ -156,12 +173,14 @@ impl<'a, 'b> FunctionBindgen<'a, 'b> { .collect::>() .join("\n"); + let tag_field = variant_tag_field_name(variant_name); + uwrite!( self.src, r#" {declarations} - switch ({op}.Tag) {{ + switch ({op}.{tag_field}) {{ {cases} default: throw new global::System.ArgumentException("invalid discriminant: " + {op}); @@ -185,6 +204,15 @@ impl<'a, 'b> FunctionBindgen<'a, 'b> { let ty = self.interface_gen.type_name_with_qualifier(ty, true); let generics_position = ty.find('<'); let lifted = self.locals.tmp("lifted"); + let accessor_names = cases + .iter() + .filter(|(_, case_ty)| { + self.interface_gen + .non_empty_type(case_ty.as_ref()) + .is_some() + }) + .map(|(case_name, _)| variant_accessor_name(&variant_name, case_name)) + .collect::>(); let cases = cases .iter() @@ -213,8 +241,11 @@ impl<'a, 'b> FunctionBindgen<'a, 'b> { String::new() }; - let method = - variant_new_func_name(&variant_name, &case_name.to_csharp_ident_upper()); + let method = variant_new_func_name( + &variant_name, + &case_name.to_csharp_ident_upper(), + &accessor_names, + ); let call = if let Some(position) = generics_position { let (ty, generics) = ty.split_at(position); @@ -650,17 +681,24 @@ impl Bindgen for FunctionBindgen<'_, '_> { Instruction::VariantLower { variant, results: lowered_types, + ty, .. - } => self.lower_variant( - &variant - .cases - .iter() - .map(|case| (case.name.deref(), case.ty)) - .collect::>(), - lowered_types, - &operands[0], - results, - ), + } => { + let variant_name = self + .interface_gen + .type_name_with_qualifier(&Type::Id(*ty), false); + self.lower_variant( + &variant_name, + &variant + .cases + .iter() + .map(|case| (case.name.deref(), case.ty)) + .collect::>(), + lowered_types, + &operands[0], + results, + ); + } Instruction::VariantLift { variant, ty, .. } => self.lift_variant( &Type::Id(*ty), @@ -819,6 +857,7 @@ impl Bindgen for FunctionBindgen<'_, '_> { result, .. } => self.lower_variant( + "Result", &[("Ok", result.ok), ("Err", result.err)], lowered_types, &operands[0], @@ -873,12 +912,13 @@ impl Bindgen for FunctionBindgen<'_, '_> { // It pins the array with the garbage collector so that it can be passed to unmanaged code. // It is required to free the pin after use which is done in the Cleanup section. self.needs_cleanup = true; + let cleanups = self.cleanups_var(); uwrite!( self.src, " var {handle} = global::System.Runtime.InteropServices.GCHandle.Alloc({list}, global::System.Runtime.InteropServices.GCHandleType.Pinned); var {ptr} = {handle}.AddrOfPinnedObject(); - cleanups.Add(()=> {handle}.Free()); + {cleanups}.Add(()=> {handle}.Free()); " ); } @@ -950,10 +990,11 @@ impl Bindgen for FunctionBindgen<'_, '_> { ); self.needs_cleanup = true; + let cleanups = self.cleanups_var(); uwrite!( self.src, " - cleanups.Add(()=> {gc_handle}.Free()); + {cleanups}.Add(()=> {gc_handle}.Free()); " ); results.push(format!("{str_ptr}.ToInt32()")); @@ -1039,6 +1080,7 @@ impl Bindgen for FunctionBindgen<'_, '_> { match realloc { None => { self.needs_cleanup = true; + let cleanups = self.cleanups_var(); self.interface_gen.csharp_gen.needs_align_stack_ptr = true; uwrite!(self.src, " @@ -1051,7 +1093,7 @@ impl Bindgen for FunctionBindgen<'_, '_> { {{ var {buffer_size} = {size} * (nuint){list}.Count; {address} = global::System.Runtime.InteropServices.NativeMemory.AlignedAlloc({buffer_size}, {align}); - cleanups.Add(() => global::System.Runtime.InteropServices.NativeMemory.AlignedFree({address})); + {cleanups}.Add(() => global::System.Runtime.InteropServices.NativeMemory.AlignedFree({address})); }} " ); @@ -1178,7 +1220,18 @@ impl Bindgen for FunctionBindgen<'_, '_> { Instruction::CallInterface { func, .. } => { let module = self.interface_gen.name; - let func_name = self.func_name.to_upper_camel_case(); + let func_name = match self.kind { + FunctionKind::Method(id) + | FunctionKind::AsyncMethod(id) + | FunctionKind::Static(id) + | FunctionKind::AsyncStatic(id) => resource_func_name( + &self.interface_gen.csharp_gen.all_resources[id] + .name + .to_upper_camel_case(), + &self.func_name.to_upper_camel_case(), + ), + _ => self.func_name.to_upper_camel_case(), + }; let interface_name = CSharp::get_class_name_from_qualified_name(module).1; let class_name_root = interface_name @@ -1238,44 +1291,45 @@ impl Bindgen for FunctionBindgen<'_, '_> { ); // Start the Async function - uwriteln!( - self.src, - r#"public static async global::System.Threading.Tasks.Task {async_func_name}({}) - {{ - var cleanups = new global::System.Collections.Generic.List(); - - "#, - func.params - .iter() - .enumerate() - .map(|(i, p)| { - let mut param_type = - self.interface_gen.type_name_with_qualifier(&p.ty, false); - - // Resource types need the Impl class to be distinguised. - match p.ty { - Type::Id(type_id) => { - let id = dealias(self.interface_gen.resolve, type_id); - - let kind = &self.interface_gen.resolve.types[id].kind; - match kind { - TypeDefKind::Handle(handle) => { - let (Handle::Own(ty) | Handle::Borrow(ty)) = handle; - param_type = - self.interface_gen.csharp_gen.all_resources - [&ty] - .export_impl_name(); - } - _ => {} + let cleanups = self.cleanups_var(); + let params_str = func + .params + .iter() + .enumerate() + .map(|(i, p)| { + let mut param_type = + self.interface_gen.type_name_with_qualifier(&p.ty, false); + + // Resource types need the Impl class to be distinguised. + match p.ty { + Type::Id(type_id) => { + let id = dealias(self.interface_gen.resolve, type_id); + + let kind = &self.interface_gen.resolve.types[id].kind; + match kind { + TypeDefKind::Handle(handle) => { + let (Handle::Own(ty) | Handle::Borrow(ty)) = handle; + param_type = + self.interface_gen.csharp_gen.all_resources[&ty] + .export_impl_name(); } + _ => {} } - _ => {} } + _ => {} + } - format!("{} {}", param_type, strip_lift(&operands[i])) - }) - .collect::>() - .join(", ") + format!("{} {}", param_type, strip_lift(&operands[i])) + }) + .collect::>() + .join(", "); + uwriteln!( + self.src, + r#"public static async global::System.Threading.Tasks.Task {async_func_name}({params_str}) + {{ + var {cleanups} = new global::System.Collections.Generic.List(); + + "# ); self.needs_cleanup = true; } @@ -1357,7 +1411,10 @@ impl Bindgen for FunctionBindgen<'_, '_> { } for (_, drop) in &self.resource_drops { - uwriteln!(self.src, "{drop}?.Dispose();"); + uwriteln!( + self.src, + "((global::System.IDisposable?){drop})?.Dispose();" + ); } } @@ -1380,15 +1437,26 @@ impl Bindgen for FunctionBindgen<'_, '_> { } if self.needs_cleanup { - self.src.insert_str(0, "var cleanups = new global::System.Collections.Generic.List(); - "); + let cleanups = self.cleanups_var(); + let cleanup = self.locals.tmp("cleanup"); + if !(InterfaceGenerator::is_async(self.kind) + && self.interface_gen.direction == Direction::Export) + { + self.src.insert_str( + 0, + &format!( + "var {cleanups} = new global::System.Collections.Generic.List(); + " + ), + ); + } uwriteln!( self.src, " - foreach (var cleanup in cleanups) + foreach (var {cleanup} in {cleanups}) {{ - cleanup(); + {cleanup}(); }}" ); } @@ -1510,16 +1578,20 @@ impl Bindgen for FunctionBindgen<'_, '_> { let is_own = matches!(handle, Handle::Own(_)); let handle = self.locals.tmp("handle"); let id = dealias(self.interface_gen.resolve, *ty); - let ResourceInfo { direction, .. } = - &self.interface_gen.csharp_gen.all_resources[&id]; + let ResourceInfo { + direction, name, .. + } = &self.interface_gen.csharp_gen.all_resources[&id]; + let resource_upper = name.to_upper_camel_case(); + let handle_prop = resource_handle_property_name(&resource_upper); + let wasm_interop = resource_wasm_interop_class_name(&resource_upper); let op = &operands[0]; - uwriteln!(self.src, "var {handle} = {op}.Handle;"); + uwriteln!(self.src, "var {handle} = {op}.{handle_prop};"); match direction { Direction::Import => { if is_own { - uwriteln!(self.src, "{op}.Handle = 0;"); + uwriteln!(self.src, "{op}.{handle_prop} = 0;"); } } Direction::Export => { @@ -1535,9 +1607,9 @@ impl Bindgen for FunctionBindgen<'_, '_> { self.src, "if ({handle} == 0) {{ var {local_rep} = {export_name}.repTable.Add({op}); - {handle} = {export_name}.WasmInterop.wasmImportResourceNew({local_rep}); + {handle} = {export_name}.{wasm_interop}.wasmImportResourceNew({local_rep}); }} - {op}.Handle = 0; + {op}.{handle_prop} = 0; " ); } else { @@ -1545,8 +1617,8 @@ impl Bindgen for FunctionBindgen<'_, '_> { self.src, "if ({handle} == 0) {{ var {local_rep} = {export_name}.repTable.Add({op}); - {handle} = {export_name}.WasmInterop.wasmImportResourceNew({local_rep}); - {op}.Handle = {handle}; + {handle} = {export_name}.{wasm_interop}.wasmImportResourceNew({local_rep}); + {op}.{handle_prop} = {handle}; }}" ); } @@ -1560,8 +1632,13 @@ impl Bindgen for FunctionBindgen<'_, '_> { let is_own = matches!(handle, Handle::Own(_)); let mut resource = self.locals.tmp("resource"); let id = dealias(self.interface_gen.resolve, *ty); - let ResourceInfo { direction, .. } = - &self.interface_gen.csharp_gen.all_resources[&id]; + let ResourceInfo { + direction, name, .. + } = &self.interface_gen.csharp_gen.all_resources[&id]; + let resource_upper = name.to_upper_camel_case(); + let handle_prop = resource_handle_property_name(&resource_upper); + let thandle_struct = resource_thandle_struct_name(&resource_upper); + let wasm_interop = resource_wasm_interop_class_name(&resource_upper); let op = &operands[0]; match direction { @@ -1572,12 +1649,12 @@ impl Bindgen for FunctionBindgen<'_, '_> { if let FunctionKind::Constructor(_) = self.kind { resource = "this".to_owned(); - uwriteln!(self.src, "{resource}.Handle = {op};"); + uwriteln!(self.src, "{resource}.{handle_prop} = {op};"); } else { let var = if is_own { "var" } else { "" }; uwriteln!( self.src, - "{var} {resource} = new {import_name}(new {import_name}.THandle({op}));" + "{var} {resource} = new {import_name}(new {import_name}.{thandle_struct}({op}));" ); } if !is_own { @@ -1593,8 +1670,8 @@ impl Bindgen for FunctionBindgen<'_, '_> { uwriteln!( self.src, "var {resource} = ({export_name}) {export_name}.repTable.Get\ - ({export_name}.WasmInterop.wasmImportResourceRep({op})); - {resource}.Handle = {op};" + ({export_name}.{wasm_interop}.wasmImportResourceRep({op})); + {resource}.{handle_prop} = {op};" ); } else { uwriteln!( @@ -1662,12 +1739,14 @@ impl Bindgen for FunctionBindgen<'_, '_> { uwriteln!(self.src, "{name}TaskReturn({});", operands.join(", ")); if self.needs_cleanup { + let cleanups = self.cleanups_var(); + let cleanup = self.locals.tmp("cleanup"); uwriteln!( self.src, " - foreach (var cleanup in cleanups) + foreach (var {cleanup} in {cleanups}) {{ - cleanup(); + {cleanup}(); }}" ); } @@ -1785,6 +1864,7 @@ impl Bindgen for FunctionBindgen<'_, '_> { match realloc { None => { self.needs_cleanup = true; + let cleanups = self.cleanups_var(); self.interface_gen.csharp_gen.needs_align_stack_ptr = true; uwrite!( self.src, @@ -1798,7 +1878,7 @@ impl Bindgen for FunctionBindgen<'_, '_> { {{ var {buffer_size} = {size} * (nuint){map}.Count; {address} = global::System.Runtime.InteropServices.NativeMemory.AlignedAlloc({buffer_size}, {align}); - cleanups.Add(() => global::System.Runtime.InteropServices.NativeMemory.AlignedFree({address})); + {cleanups}.Add(() => global::System.Runtime.InteropServices.NativeMemory.AlignedFree({address})); }} " ); diff --git a/crates/csharp/src/interface.rs b/crates/csharp/src/interface.rs index 594d50f06..b626bcced 100644 --- a/crates/csharp/src/interface.rs +++ b/crates/csharp/src/interface.rs @@ -2,7 +2,6 @@ use crate::csharp_ident::ToCSharpIdent; use crate::function::FunctionBindgen; use crate::function::ResourceInfo; use crate::world_generator::CSharp; -use heck::ToLowerCamelCase; use heck::{ToShoutySnakeCase, ToUpperCamelCase}; use std::collections::HashMap; use std::collections::HashSet; @@ -519,13 +518,16 @@ impl InterfaceGenerator<'_> { pub(crate) fn import(&mut self, import_module_name: &str, func: &Function) { let camel_name = match &func.kind { - FunctionKind::Freestanding - | FunctionKind::Static(_) - | FunctionKind::AsyncFreestanding - | FunctionKind::AsyncStatic(_) => func.item_name().to_upper_camel_case(), - FunctionKind::Method(_) | FunctionKind::AsyncMethod(_) => { + FunctionKind::Freestanding | FunctionKind::AsyncFreestanding => { func.item_name().to_upper_camel_case() } + FunctionKind::Static(id) + | FunctionKind::AsyncStatic(id) + | FunctionKind::Method(id) + | FunctionKind::AsyncMethod(id) => resource_func_name( + &self.csharp_gen.all_resources[id].name.to_upper_camel_case(), + &func.item_name().to_upper_camel_case(), + ), FunctionKind::Constructor(id) => { self.csharp_gen.all_resources[id].name.to_upper_camel_case() } @@ -753,8 +755,7 @@ impl InterfaceGenerator<'_> { .resolve .wasm_signature(AbiVariant::GuestImportAsync, func); - // TODO: the result is a tmp so could be result/1/2/.. Maybe this will fallout if we use lower_to_memory. - let async_status_var = "result"; // String::new(); + let async_status_var = bindgen.locals.tmp("result"); let requires_async_return_buffer_param = func.result.is_some(); let async_return_buffer = if requires_async_return_buffer_param { Some(bindgen.emit_allocation_for_type(&func.result.unwrap())) @@ -765,7 +766,7 @@ impl InterfaceGenerator<'_> { let csharp_param_names = func .params .iter() - .map(|param| param.name.to_lower_camel_case()) + .map(|param| param.name.to_csharp_ident()) .collect::>(); let (lower, wasm_params) = if sig.indirect_params { @@ -814,7 +815,7 @@ var {async_status_var} = {raw_name}({wasm_params}); let bindgen_src = bindgen.src; if let Some(buffer) = async_return_buffer { let ty = bindgen.result_type.expect("expected a result type"); - let (lift_expr, res) = self.lift_from_memory("address", &ty, Some(func)); + let (lift_expr, res) = self.lift_from_memory(&buffer, &ty, Some(func)); uwriteln!(src, "{}", bindgen_src); let return_type = self.type_name_with_qualifier(&ty, true); @@ -879,13 +880,16 @@ var {async_status_var} = {raw_name}({wasm_params}); pub(crate) fn export(&mut self, func: &Function, interface_key: Option<&WorldKey>) { let camel_name = match &func.kind { - FunctionKind::Freestanding - | FunctionKind::Static(_) - | FunctionKind::AsyncFreestanding - | FunctionKind::AsyncStatic(_) => func.item_name().to_upper_camel_case(), - FunctionKind::Method(_) | FunctionKind::AsyncMethod(_) => { + FunctionKind::Freestanding | FunctionKind::AsyncFreestanding => { func.item_name().to_upper_camel_case() } + FunctionKind::Static(id) + | FunctionKind::AsyncStatic(id) + | FunctionKind::Method(id) + | FunctionKind::AsyncMethod(id) => resource_func_name( + &self.csharp_gen.all_resources[id].name.to_upper_camel_case(), + &func.item_name().to_upper_camel_case(), + ), FunctionKind::Constructor(id) => { self.csharp_gen.all_resources[id].name.to_upper_camel_case() } @@ -1174,7 +1178,7 @@ var {async_status_var} = {raw_name}({wasm_params}); if self.csharp_gen.opts.generate_stub { let sig: String = self.sig_string(func, true); - let func_name = func.item_name().to_upper_camel_case(); + let func_name = &camel_name; uwrite!( self.stub, @@ -1374,11 +1378,12 @@ var {async_status_var} = {raw_name}({wasm_params}); fn print_docs(&mut self, docs: &Docs) { if let Some(docs) = &docs.contents { - let lines = docs - .trim() - .replace("<", "<") - .replace(">", ">") - .replace("*/", "*") // We can't close the comment, so if the user comment has */ we just remove the / + let mut sanitized = docs.trim().replace("<", "<").replace(">", ">"); + // We can't close the comment, so if the user comment has */ we just remove the / + while sanitized.contains("*/") { + sanitized = sanitized.replace("*/", "*"); + } + let lines = sanitized .lines() .map(|line| format!("* {line}")) .collect::>() @@ -1421,6 +1426,19 @@ var {async_status_var} = {raw_name}({wasm_params}); let docs = info.docs.clone(); self.print_docs(&docs); + let handle_prop = resource_handle_property_name(&upper_camel); + let thandle_struct = resource_thandle_struct_name(&upper_camel); + let wasm_interop = resource_wasm_interop_class_name(&upper_camel); + let (dispose_sig, dispose_bool_name, dispose_call) = if upper_camel == "Dispose" { + ( + "void global::System.IDisposable.Dispose()", + "Dispose_", + "((global::System.IDisposable)val).Dispose()", + ) + } else { + ("public void Dispose()", "Dispose", "val.Dispose()") + }; + match self.direction { Direction::Import => { let module_name = key @@ -1436,25 +1454,25 @@ var {async_status_var} = {raw_name}({wasm_params}); self.src, r#" {access} class {upper_camel}: global::System.IDisposable {{ - internal int Handle {{ get; set; }} + internal int {handle_prop} {{ get; set; }} - {access} readonly record struct THandle(int Handle); + {access} readonly record struct {thandle_struct}(int Handle); - {access} {upper_camel}(THandle handle) {{ - Handle = handle.Handle; + {access} {upper_camel}({thandle_struct} handle) {{ + {handle_prop} = handle.Handle; }} - public void Dispose() {{ - Dispose(true); + {dispose_sig} {{ + {dispose_bool_name}(true); }} [global::System.Runtime.InteropServices.DllImportAttribute("{module_name}", EntryPoint = "[resource-drop]{name}"), global::System.Runtime.InteropServices.WasmImportLinkageAttribute] private static extern void wasmImportResourceDrop(int p0); - protected virtual void Dispose(bool disposing) {{ - if (disposing && Handle != 0) {{ - wasmImportResourceDrop(Handle); - Handle = 0; + protected virtual void {dispose_bool_name}(bool disposing) {{ + if (disposing && {handle_prop} != 0) {{ + wasmImportResourceDrop({handle_prop}); + {handle_prop} = 0; }} }} "# @@ -1471,10 +1489,10 @@ var {async_status_var} = {raw_name}({wasm_params}); [global::System.Runtime.InteropServices.UnmanagedCallersOnlyAttribute(EntryPoint = "{prefix}[dtor]{name}")] {access} static unsafe void wasmExportResourceDtor{upper_camel}(int rep) {{ var val = ({qualified}) {qualified}.repTable.Remove(rep); - val.Handle = 0; + val.{handle_prop} = 0; // Note we call `Dispose` here even though the handle has already been disposed in case // the implementation has overridden `Dispose(bool)`. - val.Dispose(); + {dispose_call}; }} "# ); @@ -1503,14 +1521,14 @@ var {async_status_var} = {raw_name}({wasm_params}); r#" {access} abstract class {upper_camel}: global::System.IDisposable {{ internal static RepTable<{upper_camel}> repTable = new (); - internal int Handle {{ get; set; }} + internal int {handle_prop} {{ get; set; }} - public void Dispose() {{ - Dispose(true); - GC.SuppressFinalize(this); + {dispose_sig} {{ + {dispose_bool_name}(true); + global::System.GC.SuppressFinalize(this); }} - internal static class WasmInterop {{ + internal static class {wasm_interop} {{ [global::System.Runtime.InteropServices.DllImportAttribute("{module_name}", EntryPoint = "[resource-drop]{name}"), global::System.Runtime.InteropServices.WasmImportLinkageAttribute] internal static extern void wasmImportResourceDrop(int p0); @@ -1521,16 +1539,16 @@ var {async_status_var} = {raw_name}({wasm_params}); internal static extern int wasmImportResourceRep(int p0); }} - protected virtual void Dispose(bool disposing) {{ - if (Handle != 0) {{ - var handle = Handle; - Handle = 0; - WasmInterop.wasmImportResourceDrop(handle); + protected virtual void {dispose_bool_name}(bool disposing) {{ + if ({handle_prop} != 0) {{ + var handle = {handle_prop}; + {handle_prop} = 0; + {wasm_interop}.wasmImportResourceDrop(handle); }} }} ~{upper_camel}() {{ - Dispose(false); + {dispose_bool_name}(false); }} }} @@ -1616,14 +1634,23 @@ var {async_status_var} = {raw_name}({wasm_params}); .join(", "); let (camel_name, modifiers) = match &func.kind { - FunctionKind::Freestanding - | FunctionKind::AsyncFreestanding - | FunctionKind::Static(_) - | FunctionKind::AsyncStatic(_) => (func.item_name().to_upper_camel_case(), "static"), - - FunctionKind::Method(_) | FunctionKind::AsyncMethod(_) => { - (func.item_name().to_upper_camel_case(), "") + FunctionKind::Freestanding | FunctionKind::AsyncFreestanding => { + (func.item_name().to_upper_camel_case(), "static") } + FunctionKind::Static(id) | FunctionKind::AsyncStatic(id) => ( + resource_func_name( + &self.csharp_gen.all_resources[id].name.to_upper_camel_case(), + &func.item_name().to_upper_camel_case(), + ), + "static", + ), + FunctionKind::Method(id) | FunctionKind::AsyncMethod(id) => ( + resource_func_name( + &self.csharp_gen.all_resources[id].name.to_upper_camel_case(), + &func.item_name().to_upper_camel_case(), + ), + "", + ), FunctionKind::Constructor(id) => ( self.csharp_gen.all_resources[id].name.to_upper_camel_case(), "", @@ -1843,6 +1870,17 @@ impl<'a> CoreInterfaceGenerator<'a> for InterfaceGenerator<'a> { let name = name.to_upper_camel_case(); let tag_type = int_type(variant.tag()); let access = self.csharp_gen.access_modifier(); + let tag_field = variant_tag_field_name(&name); + let tags_class = variant_tags_class_name(&name); + + let accessor_names: Vec = variant + .cases + .iter() + .filter_map(|case| { + self.non_empty_type(case.ty.as_ref()) + .map(|_| variant_accessor_name(&name, &case.name)) + }) + .collect(); let constructors = variant .cases @@ -1850,7 +1888,8 @@ impl<'a> CoreInterfaceGenerator<'a> for InterfaceGenerator<'a> { .map(|case| { let case_name = case.name.to_csharp_ident(); let tag = case.name.to_csharp_ident_upper(); - let method_name = variant_new_func_name(&name, &tag); + let tag_const = variant_tag_const_name(tags_class, &tag); + let method_name = variant_new_func_name(&name, &tag, &accessor_names); let (parameter, argument) = if let Some(ty) = self.non_empty_type(case.ty.as_ref()) { ( @@ -1863,7 +1902,7 @@ impl<'a> CoreInterfaceGenerator<'a> for InterfaceGenerator<'a> { format!( "{access} static {name} {method_name}({parameter}) {{ - return new {name}(Tags.{tag}, {argument}); + return new {name}({tags_class}.{tag_const}, {argument}); }} " ) @@ -1876,18 +1915,19 @@ impl<'a> CoreInterfaceGenerator<'a> for InterfaceGenerator<'a> { .iter() .filter_map(|case| { self.non_empty_type(case.ty.as_ref()).map(|ty| { - let case_name = case.name.to_upper_camel_case(); + let accessor_name = variant_accessor_name(&name, &case.name); let tag = case.name.to_csharp_ident_upper(); + let tag_const = variant_tag_const_name(tags_class, &tag); let ty = self.type_name(ty); format!( - r#"{access} {ty} As{case_name} + r#"{access} {ty} {accessor_name} {{ get {{ - if (Tag == Tags.{tag}) + if ({tag_field} == {tags_class}.{tag_const}) return ({ty})value!; else - throw new global::System.ArgumentException("expected {tag}, got " + Tag); + throw new global::System.ArgumentException("expected {tag_const}, got " + {tag_field}); }} }} "# @@ -1903,7 +1943,8 @@ impl<'a> CoreInterfaceGenerator<'a> for InterfaceGenerator<'a> { .enumerate() .map(|(i, case)| { let tag = case.name.to_csharp_ident_upper(); - format!("{access} const {tag_type} {tag} = {i};") + let tag_const = variant_tag_const_name(tags_class, &tag); + format!("{access} const {tag_type} {tag_const} = {i};") }) .collect::>() .join("\n"); @@ -1912,18 +1953,18 @@ impl<'a> CoreInterfaceGenerator<'a> for InterfaceGenerator<'a> { self.src, " {access} class {name} {{ - {access} readonly {tag_type} Tag; + {access} readonly {tag_type} {tag_field}; private readonly object? value; private {name}({tag_type} tag, object? value) {{ - this.Tag = tag; + this.{tag_field} = tag; this.value = value; }} {constructors} {accessors} - {access} class Tags {{ + {access} class {tags_class} {{ {tags} }} }} @@ -2048,14 +2089,91 @@ fn needs_ptr(ty: &Type) -> bool { } } +pub fn variant_tag_field_name(variant_name: &str) -> &'static str { + if variant_name == "Tag" { "Tag_" } else { "Tag" } +} + +pub fn variant_tags_class_name(variant_name: &str) -> &'static str { + if variant_name == "Tags" { + "Tags_" + } else { + "Tags" + } +} + +pub fn variant_tag_const_name(tags_class: &str, tag: &str) -> String { + if tag == tags_class { + format!("{tag}_") + } else { + tag.to_string() + } +} + +pub fn variant_accessor_name(variant_name: &str, case_name: &str) -> String { + let accessor = format!("As{}", case_name.to_upper_camel_case()); + if accessor == variant_name { + format!("{accessor}_") + } else { + accessor + } +} + // Names the factory method for a variant case. A method can't share its name with the variant type, -// or with the `Tag` field and `Tags` class every variant has, so those cases get a suffix. -pub fn variant_new_func_name(variant_name: &String, tag: &String) -> String { - if *tag == *variant_name || tag == "Tag" || tag == "Tags" { - format!("{tag}_") // Underscores are not valid in wit identifiers so this should be safe. +// with the `Tag` field or `Tags` class every variant has, or with any `As` payload accessor +// property on the same variant, so those cases get a suffix. +pub fn variant_new_func_name(variant_name: &str, tag: &str, accessor_names: &[String]) -> String { + let tag_field = variant_tag_field_name(variant_name); + let tags_class = variant_tags_class_name(variant_name); + let mut name = tag.to_string(); + while name == variant_name + || name == tag_field + || name == tags_class + || accessor_names.iter().any(|a| a == &name) + { + name.push('_'); // Underscores are not valid in wit identifiers so this should be safe. + } + name +} + +pub fn resource_handle_property_name(resource_name: &str) -> &'static str { + if resource_name == "Handle" { + "Handle_" } else { - tag.clone() + "Handle" + } +} + +pub fn resource_thandle_struct_name(resource_name: &str) -> &'static str { + if resource_name == "THandle" { + "THandle_" + } else { + "THandle" + } +} + +pub fn resource_wasm_interop_class_name(resource_name: &str) -> &'static str { + if resource_name == "WasmInterop" { + "WasmInterop_" + } else { + "WasmInterop" + } +} + +pub fn resource_func_name(resource_name: &str, func_name: &str) -> String { + let handle_prop = resource_handle_property_name(resource_name); + let thandle_struct = resource_thandle_struct_name(resource_name); + let wasm_interop = resource_wasm_interop_class_name(resource_name); + let mut name = func_name.to_string(); + while name == resource_name + || name == handle_prop + || name == thandle_struct + || name == wasm_interop + || name == "Dispose" + || (resource_name == "Dispose" && name == "Dispose_") + { + name.push('_'); } + name } #[derive(Copy, Clone, PartialEq, Eq, Debug)] diff --git a/crates/csharp/src/lib.rs b/crates/csharp/src/lib.rs index 84e80397f..6743beea1 100644 --- a/crates/csharp/src/lib.rs +++ b/crates/csharp/src/lib.rs @@ -52,3 +52,81 @@ pub enum CSharpRuntime { NativeAOT, Mono, } + +#[cfg(test)] +mod tests { + use crate::csharp_ident::ToCSharpIdent; + use crate::interface::{ + resource_func_name, resource_handle_property_name, resource_thandle_struct_name, + resource_wasm_interop_class_name, variant_accessor_name, variant_new_func_name, + variant_tag_const_name, variant_tag_field_name, variant_tags_class_name, + }; + + #[test] + fn test_csharp_ident_keywords() { + assert_eq!("delegate".to_csharp_ident(), "@delegate"); + assert_eq!("DELEGATE".to_csharp_ident(), "@delegate"); + assert_eq!("Delegate".to_csharp_ident(), "@delegate"); + assert_eq!("class".to_csharp_ident(), "@class"); + assert_eq!("CLASS".to_csharp_ident(), "@class"); + assert_eq!("normal-name".to_csharp_ident(), "normalName"); + } + + #[test] + fn test_variant_collision_helpers() { + assert_eq!(variant_tag_field_name("Foo"), "Tag"); + assert_eq!(variant_tag_field_name("Tag"), "Tag_"); + + assert_eq!(variant_tags_class_name("Foo"), "Tags"); + assert_eq!(variant_tags_class_name("Tags"), "Tags_"); + + assert_eq!(variant_tag_const_name("Tags", "Foo"), "Foo"); + assert_eq!(variant_tag_const_name("Tags", "Tags"), "Tags_"); + assert_eq!(variant_tag_const_name("Tags_", "Tags"), "Tags"); + + assert_eq!(variant_accessor_name("Foo", "bar"), "AsBar"); + assert_eq!(variant_accessor_name("AsBar", "bar"), "AsBar_"); + + let accessors = vec!["AsTag".to_string(), "AsFoo".to_string()]; + assert_eq!( + variant_new_func_name("Ingredient", "Tag", &accessors), + "Tag_" + ); + assert_eq!( + variant_new_func_name("Ingredient", "Tags", &accessors), + "Tags_" + ); + assert_eq!( + variant_new_func_name("Ingredient", "Ingredient", &accessors), + "Ingredient_" + ); + assert_eq!( + variant_new_func_name("Ingredient", "AsFoo", &accessors), + "AsFoo_" + ); + assert_eq!(variant_new_func_name("Tag", "Tag", &accessors), "Tag__"); + assert_eq!(variant_new_func_name("Tags", "Tags", &accessors), "Tags__"); + } + + #[test] + fn test_resource_collision_helpers() { + assert_eq!(resource_handle_property_name("Foo"), "Handle"); + assert_eq!(resource_handle_property_name("Handle"), "Handle_"); + + assert_eq!(resource_thandle_struct_name("Foo"), "THandle"); + assert_eq!(resource_thandle_struct_name("THandle"), "THandle_"); + + assert_eq!(resource_wasm_interop_class_name("Foo"), "WasmInterop"); + assert_eq!( + resource_wasm_interop_class_name("WasmInterop"), + "WasmInterop_" + ); + + assert_eq!(resource_func_name("Foo", "Handle"), "Handle_"); + assert_eq!(resource_func_name("Foo", "THandle"), "THandle_"); + assert_eq!(resource_func_name("Foo", "WasmInterop"), "WasmInterop_"); + assert_eq!(resource_func_name("Foo", "Dispose"), "Dispose_"); + assert_eq!(resource_func_name("Foo", "Foo"), "Foo_"); + assert_eq!(resource_func_name("Dispose", "Dispose"), "Dispose__"); + } +} diff --git a/crates/go/src/lib.rs b/crates/go/src/lib.rs index 73625574d..6901d37bf 100644 --- a/crates/go/src/lib.rs +++ b/crates/go/src/lib.rs @@ -2556,9 +2556,13 @@ default: .join("\n"); let name = case.name.to_upper_camel_case(); + let mut case_func = name.clone(); + if matches_reserved_variant_function_name(&name) { + case_func.push('_'); + } let set_payload = if case.ty.is_some() { - format!("{VARIANT_PAYLOAD_NAME} := {value}.{name}()") + format!("{VARIANT_PAYLOAD_NAME} := {value}.{case_func}()") } else { String::new() }; diff --git a/tests/codegen/issue1731.wit b/tests/codegen/issue1731.wit new file mode 100644 index 000000000..50d547ede --- /dev/null +++ b/tests/codegen/issue1731.wit @@ -0,0 +1,124 @@ +package local:demo; + +world w { + import imported; + export exported; +} + +interface imported { + variant a { + tag(string), + tags(string), + as-tag, + } + + variant tag { + a, + tag(u32), + tags(u32), + } + + variant tags { + a, + tag(u32), + tags(u32), + } + + variant as-foo { + foo(string), + as-foo(string), + } + + resource b { + constructor(); + t-handle: func(); + dispose: func(); + wasm-interop: func(); + } + + resource handle { + constructor(); + f: func(); + } + + resource t-handle { + constructor(); + f: func(); + } + + resource wasm-interop { + constructor(); + f: func(); + } + + resource dispose { + constructor(); + f: func(); + } + + test-variants: func(x: a, y: tag, z: tags, w: as-foo) -> tuple; + test-resources: func(r1: borrow, r2: borrow, r3: borrow, r4: borrow, r5: borrow) -> tuple; + test-params: func(cleanups: list, cleanup: string, DELEGATE: string) -> string; +} + +interface exported { + /// **/ + variant a { + tag(string), + tags(string), + as-tag, + } + + variant tag { + a, + tag(u32), + tags(u32), + } + + variant tags { + a, + tag(u32), + tags(u32), + } + + variant as-foo { + foo(string), + as-foo(string), + } + + resource b { + constructor(); + take-handle: func(); + set-handle: func(); + handle: func(); + t-handle: func(); + dispose: func(); + wasm-interop: func(); + drop: func(); + on-drop: func(); + } + + resource handle { + constructor(); + f: func(); + } + + resource t-handle { + constructor(); + f: func(); + } + + resource wasm-interop { + constructor(); + f: func(); + } + + resource dispose { + constructor(); + f: func(); + } + + test-variants: func(x: a, y: tag, z: tags, w: as-foo) -> tuple; + test-resources: func(r1: borrow, r2: borrow, r3: borrow, r4: borrow, r5: borrow) -> tuple; + test-params: func(cleanups: list, cleanup: string, DELEGATE: string) -> string; +} From 0077cba38b74e91aa9355922a289906cc3303dd5 Mon Sep 17 00:00:00 2001 From: Alexander Medvedev Date: Fri, 30 Oct 2026 11:35:53 +0100 Subject: [PATCH 2/2] fix(cpp,d): disambiguate variant case/tag name collisions --- crates/cpp/src/lib.rs | 28 ++++++++++++++++++++++++---- crates/d/src/lib.rs | 1 + crates/test/src/cpp.rs | 2 +- 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/crates/cpp/src/lib.rs b/crates/cpp/src/lib.rs index 826428f0a..bb9c623c6 100644 --- a/crates/cpp/src/lib.rs +++ b/crates/cpp/src/lib.rs @@ -2249,7 +2249,10 @@ impl<'a> wit_bindgen_core::InterfaceGenerator<'a> for CppInterfaceGenerator<'a> let mut all_types = String::new(); for case in variant.cases.iter() { Self::docs(&mut self.r#gen.h_src.src, &case.docs); - let case_pascal = to_c_ident(&case.name).to_pascal_case(); + let mut case_pascal = to_c_ident(&case.name).to_pascal_case(); + if case_pascal == pascal { + case_pascal.push('_'); + } if !all_types.is_empty() { all_types += ", "; } @@ -3167,14 +3170,23 @@ impl<'a, 'b> Bindgen for FunctionBindgen<'a, 'b> { self.r#gen .type_name(&Type::Id(*var_ty), &self.namespace, Flavor::InStruct); + let var_pascal = self.r#gen.resolve.types[*var_ty] + .name + .as_ref() + .unwrap() + .to_pascal_case(); + uwriteln!(self.src, "switch ((int32_t) {}) {{", expr_to_match); for (i, ((case, (block, block_results)), payload)) in variant.cases.iter().zip(blocks).zip(payloads).enumerate() { uwriteln!(self.src, "case {}: {{", i); if case.ty.is_some() { - let case = - format!("{elem_ns}::{}", to_c_ident(&case.name).to_pascal_case()); + let mut case_pascal = to_c_ident(&case.name).to_pascal_case(); + if case_pascal == var_pascal { + case_pascal.push('_'); + } + let case = format!("{elem_ns}::{case_pascal}"); uwriteln!( self.src, "auto& {} = std::get<{case}>({}.variants).value;", @@ -3198,6 +3210,11 @@ impl<'a, 'b> Bindgen for FunctionBindgen<'a, 'b> { .drain(self.blocks.len() - variant.cases.len()..) .collect::>(); + let var_pascal = self.r#gen.resolve.types[*ty] + .name + .as_ref() + .unwrap() + .to_pascal_case(); let ty = self .r#gen .type_name(&Type::Id(*ty), &self.namespace, Flavor::InStruct); @@ -3213,7 +3230,10 @@ impl<'a, 'b> Bindgen for FunctionBindgen<'a, 'b> { for (i, (case, (block, block_results))) in variant.cases.iter().zip(blocks).enumerate() { - let tp = to_c_ident(&case.name).to_pascal_case(); + let mut tp = to_c_ident(&case.name).to_pascal_case(); + if tp == var_pascal { + tp.push('_'); + } uwriteln!(self.src, "case {i}: {{ {block}"); uwriteln!( self.src, diff --git a/crates/d/src/lib.rs b/crates/d/src/lib.rs index 71d18391b..de7239281 100644 --- a/crates/d/src/lib.rs +++ b/crates/d/src/lib.rs @@ -235,6 +235,7 @@ fn escape_d_identifier(name: &str) -> &str { "WitFlags" => "WitFlags_", "Option" => "Option_", "Result" => "Result_", + "Tag" => "Tag_", // part of the expansion of `variant` "bits" => "bits_", // part of WitFlags "borrow" => "borrow_", // part of the expansion of `resource` "rep" => "rep_", // part of the expansion of `resource` diff --git a/crates/test/src/cpp.rs b/crates/test/src/cpp.rs index 51ca2e827..8a2e887f9 100644 --- a/crates/test/src/cpp.rs +++ b/crates/test/src/cpp.rs @@ -51,7 +51,7 @@ impl LanguageMethods for Cpp { return false; } return match name { - "issue1514-6.wit" | "named-fixed-length-list.wit" => true, + "named-fixed-length-list.wit" => true, _ => false, } || config.async_; }