Skip to content

Commit 95b7e36

Browse files
committed
fix(cpp): escape cpp keywords
- remove keywords.wit from expected failures - use std::optional for ResultLift to avoid default constructor issues std::expected doesn't have a default constructor when the value type lacks one. Use std::optional wrapper to defer construction until the result is known. Fixes #1419 Signed-off-by: Bailey Hayes <[email protected]>
1 parent 37f2b1d commit 95b7e36

File tree

3 files changed

+113
-43
lines changed

3 files changed

+113
-43
lines changed

crates/c/src/lib.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4058,6 +4058,10 @@ pub fn to_c_ident(name: &str) -> String {
40584058
// variable names for option and result flattening.
40594059
"ret" => "ret_".into(),
40604060
"err" => "err_".into(),
4061+
// C standard library macros that conflict when used as identifiers
4062+
"stdin" => "stdin_".into(),
4063+
"stdout" => "stdout_".into(),
4064+
"stderr" => "stderr_".into(),
40614065
s => s.to_snake_case(),
40624066
}
40634067
}

crates/cpp/src/lib.rs

Lines changed: 109 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -721,14 +721,14 @@ fn namespace(resolve: &Resolve, owner: &TypeOwner, guest_export: bool, opts: &Op
721721
result.push(String::from("exports"));
722722
}
723723
match owner {
724-
TypeOwner::World(w) => result.push(resolve.worlds[*w].name.to_snake_case()),
724+
TypeOwner::World(w) => result.push(to_c_ident(&resolve.worlds[*w].name)),
725725
TypeOwner::Interface(i) => {
726726
let iface = &resolve.interfaces[*i];
727727
let pkg = &resolve.packages[iface.package.unwrap()];
728-
result.push(pkg.name.namespace.to_snake_case());
729-
result.push(pkg.name.name.to_snake_case());
728+
result.push(to_c_ident(&pkg.name.namespace));
729+
result.push(to_c_ident(&pkg.name.name));
730730
if let Some(name) = &iface.name {
731-
result.push(name.to_snake_case());
731+
result.push(to_c_ident(name));
732732
}
733733
}
734734
TypeOwner::None => (),
@@ -797,8 +797,23 @@ struct CppInterfaceGenerator<'a> {
797797

798798
impl CppInterfaceGenerator<'_> {
799799
fn types(&mut self, iface: InterfaceId) {
800-
let iface = &self.resolve().interfaces[iface];
801-
for (name, id) in iface.types.iter() {
800+
let iface_data = &self.resolve().interfaces[iface];
801+
802+
// First pass: emit forward declarations for all resources
803+
// This ensures resources can reference each other in method signatures
804+
for (name, id) in iface_data.types.iter() {
805+
let ty = &self.resolve().types[*id];
806+
if matches!(&ty.kind, TypeDefKind::Resource) {
807+
let pascal = name.to_upper_camel_case();
808+
let guest_import = self.gen.imported_interfaces.contains(&iface);
809+
let namespc = namespace(self.resolve, &ty.owner, !guest_import, &self.gen.opts);
810+
self.gen.h_src.change_namespace(&namespc);
811+
uwriteln!(self.gen.h_src.src, "class {pascal};");
812+
}
813+
}
814+
815+
// Second pass: emit full type definitions
816+
for (name, id) in iface_data.types.iter() {
802817
self.define_type(name, *id);
803818
}
804819
}
@@ -1055,7 +1070,7 @@ impl CppInterfaceGenerator<'_> {
10551070
""
10561071
};
10571072
res.arguments.push((
1058-
name.to_snake_case(),
1073+
to_c_ident(name),
10591074
self.type_name(param, &res.namespace, Flavor::Argument(abi_variant)) + is_pointer,
10601075
));
10611076
}
@@ -1478,6 +1493,19 @@ impl CppInterfaceGenerator<'_> {
14781493
}
14791494
TypeDefKind::Handle(Handle::Own(id)) => {
14801495
let mut typename = self.type_name(&Type::Id(*id), from_namespace, flavor);
1496+
let ty = &self.resolve.types[*id];
1497+
1498+
// Follow type aliases to find the actual resource definition
1499+
// When a resource is `use`d in another interface, we have a type alias
1500+
// with the new interface as owner. We need to follow to the original resource.
1501+
let resource_ty = match &ty.kind {
1502+
TypeDefKind::Type(Type::Id(resource_id)) => {
1503+
&self.resolve.types[*resource_id]
1504+
}
1505+
_ => ty,
1506+
};
1507+
1508+
let is_exported = self.is_exported_type(resource_ty);
14811509
match (false, flavor) {
14821510
(false, Flavor::Argument(AbiVariant::GuestImport))
14831511
| (true, Flavor::Argument(AbiVariant::GuestExport)) => {
@@ -1487,16 +1515,20 @@ impl CppInterfaceGenerator<'_> {
14871515
| (false, Flavor::Result(AbiVariant::GuestExport))
14881516
| (true, Flavor::Argument(AbiVariant::GuestImport))
14891517
| (true, Flavor::Result(AbiVariant::GuestImport)) => {
1490-
typename.push_str(&format!("::{OWNED_CLASS_NAME}"))
1518+
// Only exported resources have ::Owned typedef
1519+
if is_exported {
1520+
typename.push_str(&format!("::{OWNED_CLASS_NAME}"))
1521+
} else {
1522+
typename.push_str("&&")
1523+
}
14911524
}
14921525
(false, Flavor::Result(AbiVariant::GuestImport))
14931526
| (true, Flavor::Result(AbiVariant::GuestExport)) => (),
14941527
(_, Flavor::InStruct) => (),
14951528
(false, Flavor::BorrowedArgument) => (),
14961529
(_, _) => todo!(),
14971530
}
1498-
let ty = &self.resolve.types[*id];
1499-
if matches!(flavor, Flavor::InStruct) && self.is_exported_type(ty) {
1531+
if matches!(flavor, Flavor::InStruct) && is_exported {
15001532
typename.push_str(&format!("::{OWNED_CLASS_NAME}"))
15011533
}
15021534
typename
@@ -1532,16 +1564,22 @@ impl CppInterfaceGenerator<'_> {
15321564
self.scoped_type_name(*id, from_namespace, guest_export)
15331565
}
15341566
TypeDefKind::Option(o) => {
1567+
// Template parameters need base types without && or other decorations
1568+
let template_flavor = Flavor::InStruct;
15351569
self.gen.dependencies.needs_optional = true;
1536-
"std::optional<".to_string() + &self.type_name(o, from_namespace, flavor) + ">"
1570+
"std::optional<".to_string()
1571+
+ &self.type_name(o, from_namespace, template_flavor)
1572+
+ ">"
15371573
}
15381574
TypeDefKind::Result(r) => {
1575+
// Template parameters need base types without && or other decorations
1576+
let template_flavor = Flavor::InStruct;
15391577
let err_type = r.err.as_ref().map_or(String::from("wit::Void"), |ty| {
1540-
self.type_name(ty, from_namespace, flavor)
1578+
self.type_name(ty, from_namespace, template_flavor)
15411579
});
15421580
self.gen.dependencies.needs_expected = true;
15431581
"std::expected<".to_string()
1544-
+ &self.optional_type_name(r.ok.as_ref(), from_namespace, flavor)
1582+
+ &self.optional_type_name(r.ok.as_ref(), from_namespace, template_flavor)
15451583
+ ", "
15461584
+ &err_type
15471585
+ ">"
@@ -1662,7 +1700,7 @@ impl CppInterfaceGenerator<'_> {
16621700
uwriteln!(self.gen.h_src.src, "struct {pascal} {{");
16631701
for field in record.fields.iter() {
16641702
let typename = self.type_name(&field.ty, namespc, flavor);
1665-
let fname = field.name.to_snake_case();
1703+
let fname = to_c_ident(&field.name);
16661704
uwriteln!(self.gen.h_src.src, "{typename} {fname};");
16671705
}
16681706
uwriteln!(self.gen.h_src.src, "}};");
@@ -1671,6 +1709,8 @@ impl CppInterfaceGenerator<'_> {
16711709

16721710
fn is_exported_type(&self, ty: &TypeDef) -> bool {
16731711
if let TypeOwner::Interface(intf) = ty.owner {
1712+
// For resources used in export functions, check if the resource's owner
1713+
// interface is in imported_interfaces (which was populated during import())
16741714
!self.gen.imported_interfaces.contains(&intf)
16751715
} else {
16761716
true
@@ -1703,7 +1743,7 @@ impl<'a> wit_bindgen_core::InterfaceGenerator<'a> for CppInterfaceGenerator<'a>
17031743
for field in record.fields.iter() {
17041744
Self::docs(&mut self.gen.h_src.src, &field.docs);
17051745
let typename = self.type_name(&field.ty, &namespc, Flavor::InStruct);
1706-
let fname = field.name.to_snake_case();
1746+
let fname = to_c_ident(&field.name);
17071747
uwriteln!(self.gen.h_src.src, "{typename} {fname};");
17081748
}
17091749
uwriteln!(self.gen.h_src.src, "}};");
@@ -1722,7 +1762,7 @@ impl<'a> wit_bindgen_core::InterfaceGenerator<'a> for CppInterfaceGenerator<'a>
17221762
let guest_import = self.gen.imported_interfaces.contains(&intf);
17231763
let definition = !(guest_import);
17241764
let store = self.gen.start_new_file(Some(definition));
1725-
let mut world_name = self.gen.world.to_snake_case();
1765+
let mut world_name = to_c_ident(&self.gen.world);
17261766
world_name.push_str("::");
17271767
let namespc = namespace(self.resolve, &type_.owner, !guest_import, &self.gen.opts);
17281768
let pascal = name.to_upper_camel_case();
@@ -1884,7 +1924,7 @@ impl<'a> wit_bindgen_core::InterfaceGenerator<'a> for CppInterfaceGenerator<'a>
18841924
uwriteln!(self.gen.h_src.src, "k_None = 0,");
18851925
for (n, field) in flags.flags.iter().enumerate() {
18861926
Self::docs(&mut self.gen.h_src.src, &field.docs);
1887-
let fname = field.name.to_pascal_case();
1927+
let fname = to_c_ident(&field.name).to_pascal_case();
18881928
uwriteln!(self.gen.h_src.src, "k{fname} = (1ULL<<{n}),");
18891929
}
18901930
uwriteln!(self.gen.h_src.src, "}};");
@@ -1926,7 +1966,7 @@ impl<'a> wit_bindgen_core::InterfaceGenerator<'a> for CppInterfaceGenerator<'a>
19261966
let mut all_types = String::new();
19271967
for case in variant.cases.iter() {
19281968
Self::docs(&mut self.gen.h_src.src, &case.docs);
1929-
let case_pascal = case.name.to_pascal_case();
1969+
let case_pascal = to_c_ident(&case.name).to_pascal_case();
19301970
if !all_types.is_empty() {
19311971
all_types += ", ";
19321972
}
@@ -1985,7 +2025,7 @@ impl<'a> wit_bindgen_core::InterfaceGenerator<'a> for CppInterfaceGenerator<'a>
19852025
uwriteln!(
19862026
self.gen.h_src.src,
19872027
" k{} = {i},",
1988-
case.name.to_pascal_case(),
2028+
to_c_ident(&case.name).to_pascal_case(),
19892029
);
19902030
}
19912031
uwriteln!(self.gen.h_src.src, "}};\n");
@@ -2518,10 +2558,28 @@ impl<'a, 'b> Bindgen for FunctionBindgen<'a, 'b> {
25182558
&self.namespace,
25192559
Flavor::Argument(self.variant),
25202560
);
2521-
uwriteln!(
2522-
self.src,
2523-
"auto {var} = {tname}::Owned({tname}::ResourceRep({op}));"
2524-
);
2561+
2562+
// Check if this is an imported or exported resource
2563+
let resource_ty = &self.gen.resolve.types[*ty];
2564+
let resource_ty = match &resource_ty.kind {
2565+
TypeDefKind::Type(Type::Id(id)) => &self.gen.resolve.types[*id],
2566+
_ => resource_ty,
2567+
};
2568+
let is_exported = self.gen.is_exported_type(resource_ty);
2569+
2570+
if is_exported {
2571+
// Exported resources use ::Owned typedef
2572+
uwriteln!(
2573+
self.src,
2574+
"auto {var} = {tname}::Owned({tname}::ResourceRep({op}));"
2575+
);
2576+
} else {
2577+
// Imported resources construct from ResourceImportBase
2578+
uwriteln!(
2579+
self.src,
2580+
"auto {var} = {tname}(wit::{RESOURCE_IMPORT_BASE_CLASS_NAME}{{{op}}});"
2581+
);
2582+
}
25252583

25262584
results.push(format!("std::move({var})"))
25272585
}
@@ -2655,7 +2713,8 @@ impl<'a, 'b> Bindgen for FunctionBindgen<'a, 'b> {
26552713
uwriteln!(self.src, "case {}: {{", i);
26562714
if let Some(ty) = case.ty.as_ref() {
26572715
let ty = self.gen.type_name(ty, &self.namespace, Flavor::InStruct);
2658-
let case = format!("{elem_ns}::{}", case.name.to_pascal_case());
2716+
let case =
2717+
format!("{elem_ns}::{}", to_c_ident(&case.name).to_pascal_case());
26592718
uwriteln!(
26602719
self.src,
26612720
"{} &{} = std::get<{case}>({}.variants).value;",
@@ -2686,24 +2745,26 @@ impl<'a, 'b> Bindgen for FunctionBindgen<'a, 'b> {
26862745
let resultno = self.tmp();
26872746
let result = format!("variant{resultno}");
26882747

2689-
uwriteln!(self.src, "{ty} {result};");
2690-
26912748
let op0 = &operands[0];
26922749

2750+
// Use std::optional to avoid default constructor issues
2751+
self.gen.gen.dependencies.needs_optional = true;
2752+
uwriteln!(self.src, "std::optional<{ty}> {result}_opt;");
26932753
uwriteln!(self.src, "switch ({op0}) {{");
26942754
for (i, (case, (block, block_results))) in
26952755
variant.cases.iter().zip(blocks).enumerate()
26962756
{
2697-
let tp = case.name.clone().to_pascal_case();
2757+
let tp = to_c_ident(&case.name).to_pascal_case();
26982758
uwriteln!(self.src, "case {i}: {{ {block}");
26992759
uwriteln!(
27002760
self.src,
2701-
"{result}.variants = {ty}::{tp}{{{}}};",
2761+
"{result}_opt = {ty}{{{{{ty}::{tp}{{{}}}}}}};",
27022762
move_if_necessary(&block_results.first().cloned().unwrap_or_default())
27032763
);
27042764
uwriteln!(self.src, "}} break;");
27052765
}
27062766
uwriteln!(self.src, "}}");
2767+
uwriteln!(self.src, "{ty} {result} = std::move(*{result}_opt);");
27072768

27082769
results.push(result);
27092770
}
@@ -2745,7 +2806,15 @@ impl<'a, 'b> Bindgen for FunctionBindgen<'a, 'b> {
27452806
Flavor::InStruct
27462807
};
27472808
let ty = self.gen.type_name(payload, &self.namespace, flavor);
2748-
let bind_some = format!("{ty} {some_payload} = (std::move({op0})).value();");
2809+
// When extracting wit::string from optional as std::string_view, need .get_view()
2810+
let value_extract = if matches!(payload, Type::String)
2811+
&& matches!(flavor, Flavor::BorrowedArgument)
2812+
{
2813+
format!("(std::move({op0})).value().get_view()")
2814+
} else {
2815+
format!("(std::move({op0})).value()")
2816+
};
2817+
let bind_some = format!("{ty} {some_payload} = {value_extract};");
27492818

27502819
uwrite!(
27512820
self.src,
@@ -2775,14 +2844,14 @@ impl<'a, 'b> Bindgen for FunctionBindgen<'a, 'b> {
27752844

27762845
let tmp = self.tmp();
27772846
let resultname = self.tempname("option", tmp);
2847+
let some_value = move_if_necessary(&some_results[0]);
27782848
uwriteln!(
27792849
self.src,
27802850
"{full_type} {resultname};
27812851
if ({op0}) {{
27822852
{some}
2783-
{resultname}.emplace({});
2784-
}}",
2785-
some_results[0]
2853+
{resultname}.emplace({some_value});
2854+
}}"
27862855
);
27872856
results.push(format!("std::move({resultname})"));
27882857
}
@@ -2874,21 +2943,24 @@ impl<'a, 'b> Bindgen for FunctionBindgen<'a, 'b> {
28742943

28752944
let tmp = self.tmp();
28762945
let resultname = self.tempname("result", tmp);
2946+
// Use std::optional to avoid default constructor issues with std::expected
2947+
self.gen.gen.dependencies.needs_optional = true;
28772948
let ok_assign = if result.ok.is_some() {
2878-
format!("{resultname}.emplace({ok_result});")
2949+
format!("{resultname}_opt.emplace({full_type}({ok_result}));")
28792950
} else {
2880-
String::new()
2951+
format!("{resultname}_opt.emplace({full_type}());")
28812952
};
28822953
uwriteln!(
28832954
self.src,
2884-
"{full_type} {resultname};
2955+
"std::optional<{full_type}> {resultname}_opt;
28852956
if ({operand}==0) {{
28862957
{ok}
28872958
{ok_assign}
28882959
}} else {{
28892960
{err}
2890-
{resultname}={err_type}{{{err_result}}};
2891-
}}"
2961+
{resultname}_opt.emplace({err_type}{{{err_result}}});
2962+
}}
2963+
{full_type} {resultname} = std::move(*{resultname}_opt);"
28922964
);
28932965
results.push(resultname);
28942966
}

crates/test/src/cpp.rs

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@ impl LanguageMethods for Cpp {
5353
| "import-func.wit"
5454
| "issue573.wit"
5555
| "issue929-no-export.wit"
56-
| "keywords.wit"
5756
| "lift-lower-foreign.wit"
5857
| "lists.wit"
5958
| "multiversion"
@@ -68,13 +67,8 @@ impl LanguageMethods for Cpp {
6867
| "return-resource-from-export.wit"
6968
| "same-names1.wit"
7069
| "same-names5.wit"
71-
| "simple-http.wit"
7270
| "variants.wit"
7371
| "variants-unioning-types.wit"
74-
| "wasi-cli"
75-
| "wasi-filesystem"
76-
| "wasi-http"
77-
| "wasi-io"
7872
| "worlds-with-types.wit"
7973
| "streams.wit" => true,
8074
_ => false,

0 commit comments

Comments
 (0)