Support `__crubit::annotate(cpp_name="X")` to rename Rust function names that are C++ reserved keywords. We keep the thunk name untouched for two reasons: 1. First, it doesn't affect the C++ usage. 2. We can avoid demangling and mangling the thunk name as there is no straight-forward way to replace the name in the thunk name. PiperOrigin-RevId: 648856269 Change-Id: I3e5c1d2f6822e0a750298c0195efa183162139ba
diff --git a/bazel/llvm.bzl b/bazel/llvm.bzl index 8312583..12374f3 100644 --- a/bazel/llvm.bzl +++ b/bazel/llvm.bzl
@@ -53,7 +53,7 @@ executable = False, ) -LLVM_COMMIT_SHA = "9b8c2fae38bcff0b16d996ee002ff1e989fa23ea" +LLVM_COMMIT_SHA = "9ceb45cc191628926496bd33b4d52011ed519151" def llvm_loader_repository_dependencies(): # This *declares* the dependency, but it won't actually be *downloaded* unless it's used.
diff --git a/cc_bindings_from_rs/bindings.rs b/cc_bindings_from_rs/bindings.rs index 2144142..11ae5c9 100644 --- a/cc_bindings_from_rs/bindings.rs +++ b/cc_bindings_from_rs/bindings.rs
@@ -1107,10 +1107,13 @@ }; let fully_qualified_fn_name = FullyQualifiedName::new(tcx, def_id); - let short_fn_name = + let unqualified_rust_fn_name = fully_qualified_fn_name.name.expect("Functions are assumed to always have a name"); - let main_api_fn_name = - format_cc_ident(short_fn_name.as_str()).context("Error formatting function name")?; + let attribute = crubit_attr::get(tcx, def_id).unwrap(); + let cpp_name = attribute.cpp_name; + // The generated C++ function name. + let main_api_fn_name = format_cc_ident(cpp_name.unwrap_or(unqualified_rust_fn_name).as_str()) + .context("Error formatting function name")?; let mut main_api_prereqs = CcPrerequisites::default(); let main_api_ret_type = format_ret_ty_for_cc(db, &sig)?.into_tokens(&mut main_api_prereqs); @@ -1190,7 +1193,7 @@ }, None => None, }; - let needs_definition = short_fn_name.as_str() != thunk_name; + let needs_definition = unqualified_rust_fn_name.as_str() != thunk_name; let main_api_params = params .iter() .skip(if method_kind.has_self_param() { 1 } else { 0 }) @@ -1329,7 +1332,7 @@ let fully_qualified_fn_name = match struct_name.as_ref() { None => fully_qualified_fn_name.format_for_rs(), Some(struct_name) => { - let fn_name = make_rs_ident(short_fn_name.as_str()); + let fn_name = make_rs_ident(unqualified_rust_fn_name.as_str()); let struct_name = struct_name.format_for_rs(); quote! { #struct_name :: #fn_name } } @@ -3678,6 +3681,52 @@ }); } + #[test] + fn test_format_fn_cpp_name() { + let test_src = r#" + #![feature(register_tool)] + #![register_tool(__crubit)] + + #[no_mangle] + #[__crubit::annotate(cpp_name="Create")] + pub fn foo() {} + "#; + test_format_item(test_src, "foo", |result| { + let result = result.unwrap().unwrap(); + let main_api = &result.main_api; + assert!(main_api.prereqs.is_empty()); + + assert_rs_matches!( + result.rs_details, + quote! { + #[no_mangle] + extern "C" fn __crubit_thunk_foo() -> () { + ::rust_out::foo() + } + } + ); + + assert_cc_matches!( + main_api.tokens, + quote! { + void Create(); + } + ); + assert_cc_matches!( + result.cc_details.tokens, + quote! { + namespace __crubit_internal { + extern "C" void __crubit_thunk_foo(); + } + ... + inline void Create() { + return __crubit_internal::__crubit_thunk_foo(); + } + } + ); + }); + } + /// `test_format_item_fn_const` tests how bindings for an `const fn` are /// generated. ///
diff --git a/cc_bindings_from_rs/crubit_attr.rs b/cc_bindings_from_rs/crubit_attr.rs index 33bfc2c..f33e1a5 100644 --- a/cc_bindings_from_rs/crubit_attr.rs +++ b/cc_bindings_from_rs/crubit_attr.rs
@@ -29,6 +29,18 @@ /// For instance, /// `#[__crubit::annotate(internal_cc_type="std::basic_string<char>")]` pub cc_type: Option<Symbol>, + // The C++ name of the item. This allows us to rename Rust function names that are + // not C++-compatible like `new`. + // + // For instance, + // + // ``` + // #[__crubit::annotate(cpp_name="Create")] + // pub fn new() -> i32 {...} + // ``` + // + // will rename `new` in Rust to `Create` in C++. + pub cpp_name: Option<Symbol>, } /// Gets the `#[__crubit::annotate(...)]` attribute(s) applied to a definition. @@ -40,6 +52,7 @@ // reset in tests. The resulting test failures are very difficult. let crubit_annotate = &[Symbol::intern("__crubit"), Symbol::intern("annotate")]; let cc_type = Symbol::intern("cc_type"); + let cpp_name = Symbol::intern("cpp_name"); let mut crubit_attr = CrubitAttr::default(); // A quick note: the parsing logic is unfortunate, but such is life. We don't @@ -75,6 +88,20 @@ "Unexpected duplicate #[__crubit::annotate(cc_type=...)]" ); crubit_attr.cc_type = Some(s) + } else if arg.path == cpp_name { + let MetaItemKind::NameValue(value) = &arg.kind else { + bail!("Invalid #[__crubit::annotate(cpp_name=...)] attribute (expected =...)"); + }; + let LitKind::Str(s, _raw) = value.kind else { + bail!( + "Invalid #[__crubit::annotate(cpp_name=...)] attribute (expected =\"...\")" + ); + }; + ensure!( + crubit_attr.cpp_name.is_none(), + "Unexpected duplicate #[__crubit::annotate(cpp_name=...)]" + ); + crubit_attr.cpp_name = Some(s); } } } @@ -126,6 +153,34 @@ } #[test] + fn test_cpp_name() { + let test_src = r#" + #![feature(register_tool)] + #![register_tool(__crubit)] + #[__crubit::annotate(cpp_name = "Create")] + pub fn new() -> i32 { 0 } + "#; + run_compiler_for_testing(test_src, |tcx| { + let attr = get(tcx, find_def_id_by_name(tcx, "new")).unwrap(); + assert_eq!(attr.cpp_name.unwrap(), Symbol::intern("Create")); + }); + } + + #[test] + fn test_cpp_name_duplicated() { + let test_src = r#" + #![feature(register_tool)] + #![register_tool(__crubit)] + #[__crubit::annotate(cpp_name = "Create", cpp_name = "Create2")] + pub fn new() -> i32 { 0 } + "#; + run_compiler_for_testing(test_src, |tcx| { + let attr = get(tcx, find_def_id_by_name(tcx, "new")); + assert!(attr.is_err()); + }); + } + + #[test] fn test_cc_type_multi() { let test_src = r#" #![feature(register_tool)]