Add renaming to c++ method params

tool/Cargo.toml

@@ -12,7 +12,7 @@ keywords.workspace = true
 
 [dependencies]
 diplomat_core = { workspace = true, features = ["displaydoc", "hir"] }
-syn = { version = "2", features = [ "full", "extra-traits" ] }
+syn = { version = "2", features = ["full", "extra-traits"] }
 syn-inline-mod = "0.6.0"
 quote = "1.0"
 indenter = "0.3.3"
@@ -24,8 +24,10 @@ toml = "0.5.8"
 heck = "0.4" # conversion between naming convention
 displaydoc = "0.2"
 askama = "0.12"
+once_cell = "1.20.2"
+itertools = "0.14.0"
 
 [dev-dependencies]
-insta = { version = "1.7.1", features = [ "yaml" ] }
+insta = { version = "1.7.1", features = ["yaml"] }
 quote = "1.0"
-proc-macro2 = "1.0.79"
+proc-macro2 = "1.0.79"

tool/src/c/formatter.rs

@@ -241,6 +241,30 @@ impl<'tcx> CFormatter<'tcx> {
         )
     }
 
+    pub(crate) fn fmt_identifier<'a>(&self, name: Cow<'a, str>) -> Cow<'a, str> {
+        // TODO(#60): handle other keywords
+        // TODO: Replace with LazyLock when MSRV is bumped to >= 1.80.0
+        static C_KEYWORDS: once_cell::sync::Lazy<std::collections::HashSet<&str>> =
+            once_cell::sync::Lazy::new(|| [].into());
+
+        static CPP_KEYWORDS: once_cell::sync::Lazy<std::collections::HashSet<&str>> =
+            once_cell::sync::Lazy::new(|| ["new", "default", "delete"].into());
+
+        let lang_keywords = {
+            if self.is_for_cpp {
+                &CPP_KEYWORDS
+            } else {
+                &C_KEYWORDS
+            }
+        };
+
+        if lang_keywords.contains(name.as_ref()) {
+            format!("{name}_").into()
+        } else {
+            name
+        }
+    }
+
     fn diplomat_namespace(&self, ty: Cow<'tcx, str>) -> Cow<'tcx, str> {
         if self.is_for_cpp {
             format!("diplomat::{CAPI_NAMESPACE}::{ty}").into()

tool/src/c/ty.rs

@@ -8,7 +8,6 @@ use diplomat_core::hir::{
     SymbolId, TraitIdGetter, TyPosition, Type, TypeDef, TypeId,
 };
 use std::borrow::Cow;
-use std::fmt::Write;
 
 #[derive(Template)]
 #[template(path = "c/enum.h.jinja", escape = "none")]
@@ -74,13 +73,13 @@ pub struct TyGenContext<'cx, 'tcx> {
     pub errors: &'cx ErrorStore<'tcx, String>,
     pub is_for_cpp: bool,
     pub id: SymbolId,
-    pub decl_header_path: &'cx String,
-    pub impl_header_path: &'cx String,
+    pub decl_header_path: &'cx str,
+    pub impl_header_path: &'cx str,
 }
 
 impl<'tcx> TyGenContext<'_, 'tcx> {
     pub fn gen_enum_def(&self, def: &'tcx hir::EnumDef) -> Header {
-        let mut decl_header = Header::new(self.decl_header_path.clone(), self.is_for_cpp);
+        let mut decl_header = Header::new(self.decl_header_path.to_owned(), self.is_for_cpp);
         let ty_name = self.formatter.fmt_type_name(self.id.try_into().unwrap());
         EnumTemplate {
             ty: def,
@@ -95,7 +94,7 @@ impl<'tcx> TyGenContext<'_, 'tcx> {
     }
 
     pub fn gen_opaque_def(&self, _def: &'tcx hir::OpaqueDef) -> Header {
-        let mut decl_header = Header::new(self.decl_header_path.clone(), self.is_for_cpp);
+        let mut decl_header = Header::new(self.decl_header_path.to_owned(), self.is_for_cpp);
         let ty_name = self.formatter.fmt_type_name(self.id.try_into().unwrap());
         OpaqueTemplate {
             ty_name,
@@ -108,7 +107,7 @@ impl<'tcx> TyGenContext<'_, 'tcx> {
     }
 
     pub fn gen_struct_def<P: TyPosition>(&self, def: &'tcx hir::StructDef<P>) -> Header {
-        let mut decl_header = Header::new(self.decl_header_path.clone(), self.is_for_cpp);
+        let mut decl_header = Header::new(self.decl_header_path.to_owned(), self.is_for_cpp);
         let ty_name = self.formatter.fmt_type_name(self.id.try_into().unwrap());
         let mut fields = vec![];
         let mut cb_structs_and_defs = vec![];
@@ -134,7 +133,7 @@ impl<'tcx> TyGenContext<'_, 'tcx> {
     }
 
     pub fn gen_trait_def(&self, def: &'tcx hir::TraitDef) -> Header {
-        let mut decl_header = Header::new(self.decl_header_path.clone(), self.is_for_cpp);
+        let mut decl_header = Header::new(self.decl_header_path.to_owned(), self.is_for_cpp);
         let trt_name = self.formatter.fmt_trait_name(self.id.try_into().unwrap());
         let mut method_sigs = vec![];
         for m in &def.methods {
@@ -169,7 +168,7 @@ impl<'tcx> TyGenContext<'_, 'tcx> {
     }
 
     pub fn gen_impl(&self, ty: hir::TypeDef<'tcx>) -> Header {
-        let mut impl_header = Header::new(self.impl_header_path.clone(), self.is_for_cpp);
+        let mut impl_header = Header::new(self.impl_header_path.to_owned(), self.is_for_cpp);
         let mut methods = vec![];
         let mut cb_structs_and_defs = vec![];
         for method in ty.methods() {
@@ -203,7 +202,7 @@ impl<'tcx> TyGenContext<'_, 'tcx> {
         .render_into(&mut impl_header)
         .unwrap();
 
-        impl_header.decl_include = Some(self.decl_header_path.clone());
+        impl_header.decl_include = Some(self.decl_header_path.to_owned());
 
         // In some cases like generating decls for `self` parameters,
         // a header will get its own includes. Instead of
@@ -282,21 +281,17 @@ impl<'tcx> TyGenContext<'_, 'tcx> {
             _ => unreachable!("unknown AST/HIR variant"),
         };
 
-        let mut params = String::new();
-        let mut first = true;
-        for (decl_ty, decl_name) in param_decls {
-            let comma = if first {
-                first = false;
-                ""
-            } else {
-                ", "
-            };
-            write!(&mut params, "{comma}{decl_ty} {decl_name}").unwrap();
-        }
-
-        if params.is_empty() {
-            params.push_str("void");
-        }
+        use itertools::Itertools;
+        let params = if !param_decls.is_empty() {
+            param_decls
+                .into_iter()
+                .map(|(ty, name)| {
+                    format!("{ty} {name}", name = self.formatter.fmt_identifier(name))
+                })
+                .join(", ")
+        } else {
+            "void".to_owned()
+        };
 
         (
             MethodTemplate {

tool/src/cpp/formatter.rs

@@ -92,10 +92,11 @@ impl<'tcx> Cpp2Formatter<'tcx> {
     ) -> Cow<'tcx, str> {
         self.c.fmt_enum_variant(ctype, variant)
     }
+
     /// Format a field name or parameter name
     // might need splitting in the future if we decide to support renames here
     pub fn fmt_param_name<'a>(&self, ident: &'a str) -> Cow<'a, str> {
-        ident.into()
+        self.fmt_identifier(ident.into())
     }
 
     pub fn fmt_c_type_name(&self, id: TypeId) -> Cow<'tcx, str> {
@@ -164,16 +165,7 @@ impl<'tcx> Cpp2Formatter<'tcx> {
 
     /// Format a method
     pub fn fmt_method_name<'a>(&self, method: &'a hir::Method) -> Cow<'a, str> {
-        let name = method.attrs.rename.apply(method.name.as_str().into());
-
-        // TODO(#60): handle other keywords
-        if name == "new" {
-            "new_".into()
-        } else if name == "default" {
-            "default_".into()
-        } else {
-            name
-        }
+        self.fmt_identifier(method.attrs.rename.apply(method.name.as_str().into()))
     }
 
     pub fn namespace_c_method_name(&self, ty: TypeId, name: &str) -> String {
@@ -189,4 +181,32 @@ impl<'tcx> Cpp2Formatter<'tcx> {
     pub fn fmt_primitive_as_c(&self, prim: hir::PrimitiveType) -> Cow<'static, str> {
         self.c.fmt_primitive_as_c(prim)
     }
+
+    /// Replace any keywords used
+    pub fn fmt_identifier<'a>(&self, name: Cow<'a, str>) -> Cow<'a, str> {
+        self.c.fmt_identifier(name)
+    }
+}
+
+#[cfg(test)]
+pub mod test {
+    use super::*;
+    use proc_macro2::TokenStream;
+
+    pub fn new_tcx(tk_stream: TokenStream) -> TypeContext {
+        let file = syn::parse2::<syn::File>(tk_stream).unwrap();
+
+        let mut attr_validator = hir::BasicAttributeValidator::new("cpp_test");
+        attr_validator.support = super::super::attr_support();
+
+        match TypeContext::from_syn(&file, attr_validator) {
+            Ok(context) => context,
+            Err(e) => {
+                for (_cx, err) in e {
+                    eprintln!("Lowering error: {}", err);
+                }
+                panic!("Failed to create context")
+            }
+        }
+    }
 }

tool/src/cpp/mod.rs

@@ -103,3 +103,65 @@ pub(crate) fn run(tcx: &hir::TypeContext) -> (FileMap, ErrorStore<String>) {
 
     (files, errors)
 }
+
+#[cfg(test)]
+mod test {
+
+    use diplomat_core::hir::TypeDef;
+    use quote::quote;
+
+    use crate::cpp::header;
+    use crate::ErrorStore;
+
+    use super::{formatter::test::new_tcx, formatter::Cpp2Formatter, TyGenContext};
+
+    #[test]
+    fn test_rename_param() {
+        let tk_stream = quote! {
+            #[diplomat::bridge]
+            mod ffi {
+                #[diplomat::opaque]
+                struct MyStruct(u64);
+
+                impl MyStruct {
+                    pub fn new(&self, default: u8) {
+                        self.0 = default;
+                    }
+                }
+            }
+        };
+
+        let tcx = new_tcx(tk_stream);
+        let mut all_types = tcx.all_types();
+        if let (id, TypeDef::Opaque(opaque_def)) = all_types
+            .next()
+            .expect("Failed to generate first opaque def")
+        {
+            let error_store = ErrorStore::default();
+            let formatter = Cpp2Formatter::new(&tcx);
+            let mut decl_header = header::Header::new("decl_thing".into());
+            let mut impl_header = header::Header::new("impl_thing".into());
+
+            let mut ty_gen_cx = TyGenContext {
+                errors: &error_store,
+                formatter: &formatter,
+                c: crate::c::TyGenContext {
+                    tcx: &tcx,
+                    formatter: &formatter.c,
+                    errors: &error_store,
+                    is_for_cpp: true,
+                    id: id.into(),
+                    decl_header_path: "test/",
+                    impl_header_path: "test/",
+                },
+                decl_header: &mut decl_header,
+                impl_header: &mut impl_header,
+                generating_struct_fields: false,
+            };
+
+            ty_gen_cx.gen_opaque_def(opaque_def, id);
+            insta::assert_snapshot!(decl_header.body);
+            insta::assert_snapshot!(impl_header.body);
+        }
+    }
+}

tool/src/cpp/snapshots/diplomat_tool__cpp__test__rename_param-2.snap

@@ -0,0 +1,41 @@
+---
+source: tool/src/cpp/mod.rs
+expression: impl_header.body
+---
+namespace diplomat {
+namespace capi {
+    extern "C" {
+
+    void MyStruct_new(const diplomat::capi::MyStruct* self, uint8_t default_);
+
+
+    void MyStruct_destroy(MyStruct* self);
+
+    } // extern "C"
+} // namespace capi
+} // namespace
+
+inline void MyStruct::new_(uint8_t default_) const {
+	diplomat::capi::MyStruct_new(this->AsFFI(),
+		default_);
+}
+
+inline const diplomat::capi::MyStruct* MyStruct::AsFFI() const {
+	return reinterpret_cast<const diplomat::capi::MyStruct*>(this);
+}
+
+inline diplomat::capi::MyStruct* MyStruct::AsFFI() {
+	return reinterpret_cast<diplomat::capi::MyStruct*>(this);
+}
+
+inline const MyStruct* MyStruct::FromFFI(const diplomat::capi::MyStruct* ptr) {
+	return reinterpret_cast<const MyStruct*>(ptr);
+}
+
+inline MyStruct* MyStruct::FromFFI(diplomat::capi::MyStruct* ptr) {
+	return reinterpret_cast<MyStruct*>(ptr);
+}
+
+inline void MyStruct::operator delete(void* ptr) {
+	diplomat::capi::MyStruct_destroy(reinterpret_cast<diplomat::capi::MyStruct*>(ptr));
+}