Skip to content

Commit e6f0f3a

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 e6f0f3a

File tree

3 files changed

+125
-43
lines changed

3 files changed

+125
-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: 121 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,26 @@ 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+
// For import arguments, use BorrowedArgument flavor to get string_view
1569+
let template_flavor = match flavor {
1570+
Flavor::Argument(AbiVariant::GuestImport) => Flavor::BorrowedArgument,
1571+
_ => Flavor::InStruct,
1572+
};
15351573
self.gen.dependencies.needs_optional = true;
1536-
"std::optional<".to_string() + &self.type_name(o, from_namespace, flavor) + ">"
1574+
"std::optional<".to_string()
1575+
+ &self.type_name(o, from_namespace, template_flavor)
1576+
+ ">"
15371577
}
15381578
TypeDefKind::Result(r) => {
1579+
// Template parameters need base types without && or other decorations
1580+
let template_flavor = Flavor::InStruct;
15391581
let err_type = r.err.as_ref().map_or(String::from("wit::Void"), |ty| {
1540-
self.type_name(ty, from_namespace, flavor)
1582+
self.type_name(ty, from_namespace, template_flavor)
15411583
});
15421584
self.gen.dependencies.needs_expected = true;
15431585
"std::expected<".to_string()
1544-
+ &self.optional_type_name(r.ok.as_ref(), from_namespace, flavor)
1586+
+ &self.optional_type_name(r.ok.as_ref(), from_namespace, template_flavor)
15451587
+ ", "
15461588
+ &err_type
15471589
+ ">"
@@ -1662,7 +1704,7 @@ impl CppInterfaceGenerator<'_> {
16621704
uwriteln!(self.gen.h_src.src, "struct {pascal} {{");
16631705
for field in record.fields.iter() {
16641706
let typename = self.type_name(&field.ty, namespc, flavor);
1665-
let fname = field.name.to_snake_case();
1707+
let fname = to_c_ident(&field.name);
16661708
uwriteln!(self.gen.h_src.src, "{typename} {fname};");
16671709
}
16681710
uwriteln!(self.gen.h_src.src, "}};");
@@ -1671,6 +1713,8 @@ impl CppInterfaceGenerator<'_> {
16711713

16721714
fn is_exported_type(&self, ty: &TypeDef) -> bool {
16731715
if let TypeOwner::Interface(intf) = ty.owner {
1716+
// For resources used in export functions, check if the resource's owner
1717+
// interface is in imported_interfaces (which was populated during import())
16741718
!self.gen.imported_interfaces.contains(&intf)
16751719
} else {
16761720
true
@@ -1703,7 +1747,7 @@ impl<'a> wit_bindgen_core::InterfaceGenerator<'a> for CppInterfaceGenerator<'a>
17031747
for field in record.fields.iter() {
17041748
Self::docs(&mut self.gen.h_src.src, &field.docs);
17051749
let typename = self.type_name(&field.ty, &namespc, Flavor::InStruct);
1706-
let fname = field.name.to_snake_case();
1750+
let fname = to_c_ident(&field.name);
17071751
uwriteln!(self.gen.h_src.src, "{typename} {fname};");
17081752
}
17091753
uwriteln!(self.gen.h_src.src, "}};");
@@ -1722,7 +1766,7 @@ impl<'a> wit_bindgen_core::InterfaceGenerator<'a> for CppInterfaceGenerator<'a>
17221766
let guest_import = self.gen.imported_interfaces.contains(&intf);
17231767
let definition = !(guest_import);
17241768
let store = self.gen.start_new_file(Some(definition));
1725-
let mut world_name = self.gen.world.to_snake_case();
1769+
let mut world_name = to_c_ident(&self.gen.world);
17261770
world_name.push_str("::");
17271771
let namespc = namespace(self.resolve, &type_.owner, !guest_import, &self.gen.opts);
17281772
let pascal = name.to_upper_camel_case();
@@ -1884,7 +1928,7 @@ impl<'a> wit_bindgen_core::InterfaceGenerator<'a> for CppInterfaceGenerator<'a>
18841928
uwriteln!(self.gen.h_src.src, "k_None = 0,");
18851929
for (n, field) in flags.flags.iter().enumerate() {
18861930
Self::docs(&mut self.gen.h_src.src, &field.docs);
1887-
let fname = field.name.to_pascal_case();
1931+
let fname = to_c_ident(&field.name).to_pascal_case();
18881932
uwriteln!(self.gen.h_src.src, "k{fname} = (1ULL<<{n}),");
18891933
}
18901934
uwriteln!(self.gen.h_src.src, "}};");
@@ -1926,7 +1970,7 @@ impl<'a> wit_bindgen_core::InterfaceGenerator<'a> for CppInterfaceGenerator<'a>
19261970
let mut all_types = String::new();
19271971
for case in variant.cases.iter() {
19281972
Self::docs(&mut self.gen.h_src.src, &case.docs);
1929-
let case_pascal = case.name.to_pascal_case();
1973+
let case_pascal = to_c_ident(&case.name).to_pascal_case();
19301974
if !all_types.is_empty() {
19311975
all_types += ", ";
19321976
}
@@ -1985,7 +2029,7 @@ impl<'a> wit_bindgen_core::InterfaceGenerator<'a> for CppInterfaceGenerator<'a>
19852029
uwriteln!(
19862030
self.gen.h_src.src,
19872031
" k{} = {i},",
1988-
case.name.to_pascal_case(),
2032+
to_c_ident(&case.name).to_pascal_case(),
19892033
);
19902034
}
19912035
uwriteln!(self.gen.h_src.src, "}};\n");
@@ -2518,10 +2562,28 @@ impl<'a, 'b> Bindgen for FunctionBindgen<'a, 'b> {
25182562
&self.namespace,
25192563
Flavor::Argument(self.variant),
25202564
);
2521-
uwriteln!(
2522-
self.src,
2523-
"auto {var} = {tname}::Owned({tname}::ResourceRep({op}));"
2524-
);
2565+
2566+
// Check if this is an imported or exported resource
2567+
let resource_ty = &self.gen.resolve.types[*ty];
2568+
let resource_ty = match &resource_ty.kind {
2569+
TypeDefKind::Type(Type::Id(id)) => &self.gen.resolve.types[*id],
2570+
_ => resource_ty,
2571+
};
2572+
let is_exported = self.gen.is_exported_type(resource_ty);
2573+
2574+
if is_exported {
2575+
// Exported resources use ::Owned typedef
2576+
uwriteln!(
2577+
self.src,
2578+
"auto {var} = {tname}::Owned({tname}::ResourceRep({op}));"
2579+
);
2580+
} else {
2581+
// Imported resources construct from ResourceImportBase
2582+
uwriteln!(
2583+
self.src,
2584+
"auto {var} = {tname}(wit::{RESOURCE_IMPORT_BASE_CLASS_NAME}{{{op}}});"
2585+
);
2586+
}
25252587

25262588
results.push(format!("std::move({var})"))
25272589
}
@@ -2655,7 +2717,8 @@ impl<'a, 'b> Bindgen for FunctionBindgen<'a, 'b> {
26552717
uwriteln!(self.src, "case {}: {{", i);
26562718
if let Some(ty) = case.ty.as_ref() {
26572719
let ty = self.gen.type_name(ty, &self.namespace, Flavor::InStruct);
2658-
let case = format!("{elem_ns}::{}", case.name.to_pascal_case());
2720+
let case =
2721+
format!("{elem_ns}::{}", to_c_ident(&case.name).to_pascal_case());
26592722
uwriteln!(
26602723
self.src,
26612724
"{} &{} = std::get<{case}>({}.variants).value;",
@@ -2686,24 +2749,26 @@ impl<'a, 'b> Bindgen for FunctionBindgen<'a, 'b> {
26862749
let resultno = self.tmp();
26872750
let result = format!("variant{resultno}");
26882751

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

2754+
// Use std::optional to avoid default constructor issues
2755+
self.gen.gen.dependencies.needs_optional = true;
2756+
uwriteln!(self.src, "std::optional<{ty}> {result}_opt;");
26932757
uwriteln!(self.src, "switch ({op0}) {{");
26942758
for (i, (case, (block, block_results))) in
26952759
variant.cases.iter().zip(blocks).enumerate()
26962760
{
2697-
let tp = case.name.clone().to_pascal_case();
2761+
let tp = to_c_ident(&case.name).to_pascal_case();
26982762
uwriteln!(self.src, "case {i}: {{ {block}");
26992763
uwriteln!(
27002764
self.src,
2701-
"{result}.variants = {ty}::{tp}{{{}}};",
2765+
"{result}_opt = {ty}{{{{{ty}::{tp}{{{}}}}}}};",
27022766
move_if_necessary(&block_results.first().cloned().unwrap_or_default())
27032767
);
27042768
uwriteln!(self.src, "}} break;");
27052769
}
27062770
uwriteln!(self.src, "}}");
2771+
uwriteln!(self.src, "{ty} {result} = std::move(*{result}_opt);");
27072772

27082773
results.push(result);
27092774
}
@@ -2745,7 +2810,23 @@ impl<'a, 'b> Bindgen for FunctionBindgen<'a, 'b> {
27452810
Flavor::InStruct
27462811
};
27472812
let ty = self.gen.type_name(payload, &self.namespace, flavor);
2748-
let bind_some = format!("{ty} {some_payload} = (std::move({op0})).value();");
2813+
// For string lowering, we need to determine if .get_view() is needed:
2814+
// - GuestImport lowering: source optional (from struct/param) may contain wit::string,
2815+
// destination needs std::string_view, so call .get_view() to convert
2816+
// - GuestExport lowering: source is from ABI (std::string_view),
2817+
// destination is wit::string or std::string_view depending on context,
2818+
// so no .get_view() needed (wit::string constructor accepts string_view)
2819+
let value_extract = if matches!(payload, Type::String)
2820+
&& matches!(self.variant, AbiVariant::GuestImport)
2821+
{
2822+
// Import: optional may contain wit::string (from struct field)
2823+
// Need .get_view() to convert to string_view for ABI
2824+
format!("(std::move({op0})).value().get_view()")
2825+
} else {
2826+
// Export or non-string: just .value()
2827+
format!("(std::move({op0})).value()")
2828+
};
2829+
let bind_some = format!("{ty} {some_payload} = {value_extract};");
27492830

27502831
uwrite!(
27512832
self.src,
@@ -2775,14 +2856,14 @@ impl<'a, 'b> Bindgen for FunctionBindgen<'a, 'b> {
27752856

27762857
let tmp = self.tmp();
27772858
let resultname = self.tempname("option", tmp);
2859+
let some_value = move_if_necessary(&some_results[0]);
27782860
uwriteln!(
27792861
self.src,
27802862
"{full_type} {resultname};
27812863
if ({op0}) {{
27822864
{some}
2783-
{resultname}.emplace({});
2784-
}}",
2785-
some_results[0]
2865+
{resultname}.emplace({some_value});
2866+
}}"
27862867
);
27872868
results.push(format!("std::move({resultname})"));
27882869
}
@@ -2874,21 +2955,24 @@ impl<'a, 'b> Bindgen for FunctionBindgen<'a, 'b> {
28742955

28752956
let tmp = self.tmp();
28762957
let resultname = self.tempname("result", tmp);
2958+
// Use std::optional to avoid default constructor issues with std::expected
2959+
self.gen.gen.dependencies.needs_optional = true;
28772960
let ok_assign = if result.ok.is_some() {
2878-
format!("{resultname}.emplace({ok_result});")
2961+
format!("{resultname}_opt.emplace({full_type}({ok_result}));")
28792962
} else {
2880-
String::new()
2963+
format!("{resultname}_opt.emplace({full_type}());")
28812964
};
28822965
uwriteln!(
28832966
self.src,
2884-
"{full_type} {resultname};
2967+
"std::optional<{full_type}> {resultname}_opt;
28852968
if ({operand}==0) {{
28862969
{ok}
28872970
{ok_assign}
28882971
}} else {{
28892972
{err}
2890-
{resultname}={err_type}{{{err_result}}};
2891-
}}"
2973+
{resultname}_opt.emplace({err_type}{{{err_result}}});
2974+
}}
2975+
{full_type} {resultname} = std::move(*{resultname}_opt);"
28922976
);
28932977
results.push(resultname);
28942978
}

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)