Partially-support nested type aliases: use the underlying type instead. This allows for functions and types which use those type aliases to be used, even if the type aliases themselves can't be used directly. This should help unblock the use of STL containers like `string_view`, where a bunch of member functions talk about the types `basic_string_view::size_type`, or `basic_string_view::value_type`, or `const_iterator`, etc. For example, string_view::size now has unsafe bindings, but did not before. PiperOrigin-RevId: 474279132
diff --git a/rs_bindings_from_cc/importers/typedef_name.cc b/rs_bindings_from_cc/importers/typedef_name.cc index eeccfd1..54b4cbd 100644 --- a/rs_bindings_from_cc/importers/typedef_name.cc +++ b/rs_bindings_from_cc/importers/typedef_name.cc
@@ -7,20 +7,20 @@ #include "absl/log/check.h" #include "clang/AST/ASTContext.h" #include "clang/AST/Attr.h" +#include "clang/AST/Decl.h" namespace crubit { std::optional<IR::Item> crubit::TypedefNameDeclImporter::Import( clang::TypedefNameDecl* typedef_name_decl) { const clang::DeclContext* decl_context = typedef_name_decl->getDeclContext(); + llvm::Optional<ItemId> enclosing_record_id = llvm::None; if (decl_context) { if (decl_context->isFunctionOrMethod()) { return std::nullopt; } - if (decl_context->isRecord()) { - return ictx_.ImportUnsupportedItem( - typedef_name_decl, - "Typedefs nested in classes are not supported yet"); + if (auto* record_decl = llvm::dyn_cast<clang::RecordDecl>(decl_context)) { + enclosing_record_id = GenerateItemId(record_decl); } } @@ -53,7 +53,11 @@ .owning_target = ictx_.GetOwningTarget(typedef_name_decl), .doc_comment = ictx_.GetComment(typedef_name_decl), .underlying_type = *underlying_type, - .enclosing_namespace_id = GetEnclosingNamespaceId(typedef_name_decl)}; + .source_loc = + ictx_.ConvertSourceLocation(typedef_name_decl->getBeginLoc()), + .enclosing_record_id = enclosing_record_id, + .enclosing_namespace_id = GetEnclosingNamespaceId(typedef_name_decl), + }; } else if (typedef_name_decl->getAnonDeclWithTypedefName()) { auto* type = typedef_name_decl->getUnderlyingType().getTypePtrOrNull(); if (type && type->getAsRecordDecl()) {
diff --git a/rs_bindings_from_cc/ir.cc b/rs_bindings_from_cc/ir.cc index 7869e42..90641f5 100644 --- a/rs_bindings_from_cc/ir.cc +++ b/rs_bindings_from_cc/ir.cc
@@ -458,6 +458,8 @@ {"owning_target", owning_target}, {"doc_comment", doc_comment}, {"underlying_type", underlying_type}, + {"source_loc", source_loc}, + {"enclosing_record_id", enclosing_record_id}, {"enclosing_namespace_id", enclosing_namespace_id}, };
diff --git a/rs_bindings_from_cc/ir.h b/rs_bindings_from_cc/ir.h index 868f6ac..d702984 100644 --- a/rs_bindings_from_cc/ir.h +++ b/rs_bindings_from_cc/ir.h
@@ -703,6 +703,8 @@ BazelLabel owning_target; llvm::Optional<std::string> doc_comment; MappedType underlying_type; + SourceLoc source_loc; + llvm::Optional<ItemId> enclosing_record_id; llvm::Optional<ItemId> enclosing_namespace_id; };
diff --git a/rs_bindings_from_cc/ir.rs b/rs_bindings_from_cc/ir.rs index bb10ce6..99c49c4 100644 --- a/rs_bindings_from_cc/ir.rs +++ b/rs_bindings_from_cc/ir.rs
@@ -487,6 +487,8 @@ pub owning_target: BazelLabel, pub doc_comment: Option<String>, pub underlying_type: MappedType, + pub source_loc: SourceLoc, + pub enclosing_record_id: Option<ItemId>, pub enclosing_namespace_id: Option<ItemId>, } @@ -519,6 +521,7 @@ pub owning_target: BazelLabel, #[serde(default)] pub child_item_ids: Vec<ItemId>, + pub enclosing_record_id: Option<ItemId>, pub enclosing_namespace_id: Option<ItemId>, pub is_inline: bool, } @@ -794,10 +797,9 @@ /// `self`, returns an error. pub fn record_for_member_func<'a>(&self, func: &'a Func) -> Result<Option<&Rc<Record>>> { if let Some(meta) = func.member_func_metadata.as_ref() { - Ok(Some( - self.find_decl(meta.record_id) - .with_context(|| format!("Failed to retrieve Record for MemberFuncMetadata of {:?}", func))?, - )) + Ok(Some(self.find_decl(meta.record_id).with_context(|| { + format!("Failed to retrieve Record for MemberFuncMetadata of {:?}", func) + })?)) } else { Ok(None) }
diff --git a/rs_bindings_from_cc/ir_from_cc_test.rs b/rs_bindings_from_cc/ir_from_cc_test.rs index 62f366c..96775ec 100644 --- a/rs_bindings_from_cc/ir_from_cc_test.rs +++ b/rs_bindings_from_cc/ir_from_cc_test.rs
@@ -708,22 +708,8 @@ let type_mapping: HashMap<_, _> = fields .map(|f| { ( - f.type_ - .as_ref() - .unwrap() - .cc_type - .name - .as_ref() - .unwrap() - .as_str(), - f.type_ - .as_ref() - .unwrap() - .rs_type - .name - .as_ref() - .unwrap() - .as_str(), + f.type_.as_ref().unwrap().cc_type.name.as_ref().unwrap().as_str(), + f.type_.as_ref().unwrap().rs_type.name.as_ref().unwrap().as_str(), ) }) .collect(); @@ -824,6 +810,8 @@ owning_target: BazelLabel("//test:testing_target"), doc_comment: Some("Doc comment for MyTypedefDecl."), underlying_type: #int, + source_loc: SourceLoc {...}, + enclosing_record_id: None, enclosing_namespace_id: None, } } @@ -837,6 +825,8 @@ owning_target: BazelLabel("//test:testing_target"), doc_comment: Some("Doc comment for MyTypeAliasDecl."), underlying_type: #int, + source_loc: SourceLoc {...}, + enclosing_record_id: None, enclosing_namespace_id: None, } } @@ -2093,18 +2083,6 @@ } #[test] -fn test_typedef_nested_in_record_not_supported() { - let ir = ir_from_cc("struct S { typedef int MyTypedefDecl; };").unwrap(); - assert_strings_contain( - ir.unsupported_items() - .map(|i| i.name.as_str()) - .collect_vec() - .as_slice(), - "S::MyTypedefDecl", - ); -} - -#[test] fn test_records_nested_in_records_not_supported_yet() { let ir = ir_from_cc("struct SomeStruct { struct NestedStruct {}; };").unwrap(); assert_ir_matches!( @@ -2681,10 +2659,7 @@ // once we start importing nested structs. let ir = ir_from_cc("struct X { struct Y {}; };")?; assert_strings_contain( - ir.unsupported_items() - .map(|i| i.name.as_str()) - .collect_vec() - .as_slice(), + ir.unsupported_items().map(|i| i.name.as_str()).collect_vec().as_slice(), "X::Y", ); Ok(())
diff --git a/rs_bindings_from_cc/src_code_gen.rs b/rs_bindings_from_cc/src_code_gen.rs index 267fc51..f0bd8fe 100644 --- a/rs_bindings_from_cc/src_code_gen.rs +++ b/rs_bindings_from_cc/src_code_gen.rs
@@ -326,6 +326,16 @@ }) } +fn make_unsupported_nested_type_alias(type_alias: &TypeAlias) -> Result<UnsupportedItem> { + Ok(UnsupportedItem { + // TODO(jeanpierreda): It would be nice to include the enclosing record name here too. + name: type_alias.identifier.identifier.to_string(), + message: "Typedefs nested in classes are not supported yet".to_string(), + source_loc: type_alias.source_loc.clone(), + id: type_alias.id, + }) +} + /// The name of a one-function trait, with extra entries for /// specially-understood traits and families of traits. enum TraitName { @@ -2278,6 +2288,12 @@ && !ir.is_stdlib_target(&type_alias.owning_target) { GeneratedItem::default() + } else if type_alias.enclosing_record_id.is_some() { + // TODO(b/200067824): support nested type aliases. + GeneratedItem { + item: generate_unsupported(&make_unsupported_nested_type_alias(type_alias)?)?, + ..Default::default() + } } else { GeneratedItem { item: generate_type_alias(db, type_alias)?, ..Default::default() } } @@ -2915,14 +2931,25 @@ crate_ident: rs_imported_crate_name(&incomplete_record.owning_target, &ir), }, Item::Record(record) => RsTypeKind::new_record(record.clone(), &ir)?, - Item::TypeAlias(type_alias) => RsTypeKind::TypeAlias { - type_alias: type_alias.clone(), - namespace_qualifier: Rc::new(NamespaceQualifier::new(type_alias.id, &ir)?), - crate_ident: rs_imported_crate_name(&type_alias.owning_target, &ir), - underlying_type: Rc::new( - db.rs_type_kind(type_alias.underlying_type.rs_type.clone())?, - ), - }, + Item::TypeAlias(type_alias) => { + // TODO(b/200067824): support nested type aliases. + if type_alias.enclosing_record_id.is_some() { + // Until this is supported, we import this as the underlying type. + db.rs_type_kind(type_alias.underlying_type.rs_type.clone())? + } else { + RsTypeKind::TypeAlias { + type_alias: type_alias.clone(), + namespace_qualifier: Rc::new(NamespaceQualifier::new( + type_alias.id, + &ir, + )?), + crate_ident: rs_imported_crate_name(&type_alias.owning_target, &ir), + underlying_type: Rc::new( + db.rs_type_kind(type_alias.underlying_type.rs_type.clone())?, + ), + } + } + } other_item => bail!("Item does not define a type: {:?}", other_item), } } @@ -2978,10 +3005,15 @@ } fn cc_type_name_for_record(record: &Record, ir: &IR) -> Result<TokenStream> { + let tagless = cc_tagless_type_name_for_record(record, ir)?; + let tag_kind = cc_tag_kind(record); + Ok(quote! { #tag_kind #tagless }) +} + +fn cc_tagless_type_name_for_record(record: &Record, ir: &IR) -> Result<TokenStream> { let ident = format_cc_ident(&record.cc_name); let namespace_qualifier = NamespaceQualifier::new(record.id, ir)?.format_for_cc(); - let tag_kind = cc_tag_kind(record); - Ok(quote! { #tag_kind #namespace_qualifier #ident }) + Ok(quote! { #namespace_qualifier #ident }) } fn cc_type_name_for_item(item: &ir::Item, ir: &IR) -> Result<TokenStream> { @@ -2996,8 +3028,15 @@ Item::Record(record) => cc_type_name_for_record(record, ir), Item::TypeAlias(type_alias) => { let ident = format_cc_ident(&type_alias.identifier.identifier); - let namespace_qualifier = NamespaceQualifier::new(type_alias.id, ir)?.format_for_cc(); - Ok(quote! { #namespace_qualifier #ident }) + if let Some(record_id) = type_alias.enclosing_record_id { + let parent = + cc_tagless_type_name_for_record(ir.find_decl::<Rc<Record>>(record_id)?, ir)?; + Ok(quote! { #parent :: #ident }) + } else { + let namespace_qualifier = + NamespaceQualifier::new(type_alias.id, ir)?.format_for_cc(); + Ok(quote! { #namespace_qualifier #ident }) + } } _ => bail!("Item does not define a type: {:?}", item), } @@ -3867,6 +3906,29 @@ } #[test] + fn test_typedef_member() -> Result<()> { + let ir = ir_from_cc( + r#" + struct SomeStruct final { + typedef int Type; + }; + inline SomeStruct::Type Function() {return 0;} + "#, + )?; + let BindingsTokens { rs_api, rs_api_impl } = generate_bindings_tokens(ir)?; + // TODO(b/200067824): This should use the alias's real name in Rust, as well. + assert_rs_matches!(rs_api, quote! { pub fn Function() -> i32 { ... } },); + + assert_cc_matches!( + rs_api_impl, + quote! { + extern "C" SomeStruct::Type __rust_thunk___Z8Functionv(){ return Function(); } + }, + ); + Ok(()) + } + + #[test] fn test_ref_to_struct_in_thunk_impls() -> Result<()> { let ir = ir_from_cc("struct S{}; inline void foo(S& s) {} ")?; let rs_api_impl = generate_bindings_tokens(ir)?.rs_api_impl;
diff --git a/rs_bindings_from_cc/test/golden/typedefs.h b/rs_bindings_from_cc/test/golden/typedefs.h index 2a20ed4..1e3439a 100644 --- a/rs_bindings_from_cc/test/golden/typedefs.h +++ b/rs_bindings_from_cc/test/golden/typedefs.h
@@ -7,7 +7,9 @@ #pragma clang lifetime_elision -struct SomeStruct {}; +struct SomeStruct { + typedef int nested_type; +}; typedef struct SomeStruct SomeStruct; typedef struct { @@ -19,4 +21,6 @@ typedef union { } SomeOtherUnion; +SomeStruct::nested_type FunctionUsingNestedType(); + #endif // THIRD_PARTY_CRUBIT_RS_BINDINGS_FROM_CC_TEST_GOLDEN_TYPEDEFS_H_
diff --git a/rs_bindings_from_cc/test/golden/typedefs_rs_api.rs b/rs_bindings_from_cc/test/golden/typedefs_rs_api.rs index 31bb389..ffd051a 100644 --- a/rs_bindings_from_cc/test/golden/typedefs_rs_api.rs +++ b/rs_bindings_from_cc/test/golden/typedefs_rs_api.rs
@@ -113,6 +113,10 @@ } // rs_bindings_from_cc/test/golden/typedefs.h;l=11 +// Error while generating bindings for item 'nested_type': +// Typedefs nested in classes are not supported yet + +// rs_bindings_from_cc/test/golden/typedefs.h;l=13 // Error while generating bindings for item 'SomeStruct': // Typedef only used to introduce a name in C. Not importing. @@ -255,15 +259,15 @@ } } -// rs_bindings_from_cc/test/golden/typedefs.h;l=16 +// rs_bindings_from_cc/test/golden/typedefs.h;l=18 // Error while generating bindings for item 'SomeUnion::operator=': // operator= for Unpin types is not yet supported. -// rs_bindings_from_cc/test/golden/typedefs.h;l=16 +// rs_bindings_from_cc/test/golden/typedefs.h;l=18 // Error while generating bindings for item 'SomeUnion::operator=': // operator= for Unpin types is not yet supported. -// rs_bindings_from_cc/test/golden/typedefs.h;l=17 +// rs_bindings_from_cc/test/golden/typedefs.h;l=19 // Error while generating bindings for item 'SomeUnion': // Typedef only used to introduce a name in C. Not importing. @@ -296,14 +300,19 @@ } } -// rs_bindings_from_cc/test/golden/typedefs.h;l=19 +// rs_bindings_from_cc/test/golden/typedefs.h;l=21 // Error while generating bindings for item 'SomeOtherUnion::operator=': // operator= for Unpin types is not yet supported. -// rs_bindings_from_cc/test/golden/typedefs.h;l=19 +// rs_bindings_from_cc/test/golden/typedefs.h;l=21 // Error while generating bindings for item 'SomeOtherUnion::operator=': // operator= for Unpin types is not yet supported. +#[inline(always)] +pub fn FunctionUsingNestedType() -> i32 { + unsafe { crate::detail::__rust_thunk___Z23FunctionUsingNestedTypev() } +} + // THIRD_PARTY_CRUBIT_RS_BINDINGS_FROM_CC_TEST_GOLDEN_TYPEDEFS_H_ mod detail { @@ -362,6 +371,8 @@ __this: &'a mut ::std::mem::MaybeUninit<crate::SomeOtherUnion>, __param_0: ::ctor::RvalueReference<'b, crate::SomeOtherUnion>, ); + #[link_name = "_Z23FunctionUsingNestedTypev"] + pub(crate) fn __rust_thunk___Z23FunctionUsingNestedTypev() -> i32; } }
diff --git a/rs_bindings_from_cc/test/golden/types_rs_api.rs b/rs_bindings_from_cc/test/golden/types_rs_api.rs index 185caa5..b4f287d 100644 --- a/rs_bindings_from_cc/test/golden/types_rs_api.rs +++ b/rs_bindings_from_cc/test/golden/types_rs_api.rs
@@ -186,20 +186,16 @@ // Parameter #0 is not supported: Unsupported type 'struct std::integral_constant<_Bool, false> &&': Unsupported type: && without lifetime // google3/nowhere/llvm/toolchain/include/c++/v1/__type_traits/integral_constant.h;l=24 -// Error while generating bindings for item 'std::integral_constant<bool, false>::value_type': +// Error while generating bindings for item 'value_type': // Typedefs nested in classes are not supported yet // google3/nowhere/llvm/toolchain/include/c++/v1/__type_traits/integral_constant.h;l=25 -// Error while generating bindings for item 'std::integral_constant<bool, false>::type': +// Error while generating bindings for item 'type': // Typedefs nested in classes are not supported yet // <unknown location> -// Error while generating bindings for item 'std::integral_constant<bool, false>::operator bool': -// Return type is not supported: Unsupported type 'std::integral_constant<_Bool, false>::value_type': No generated bindings found for 'value_type' - -// <unknown location> // Error while generating bindings for item 'std::integral_constant<bool, false>::operator()': -// Return type is not supported: Unsupported type 'std::integral_constant<_Bool, false>::value_type': No generated bindings found for 'value_type' +// Bindings for this kind of operator (operator () with 1 parameter(s)) are not supported #[::ctor::recursively_pinned] #[repr(C)] @@ -232,20 +228,16 @@ // Parameter #0 is not supported: Unsupported type 'struct std::integral_constant<_Bool, true> &&': Unsupported type: && without lifetime // google3/nowhere/llvm/toolchain/include/c++/v1/__type_traits/integral_constant.h;l=24 -// Error while generating bindings for item 'std::integral_constant<bool, true>::value_type': +// Error while generating bindings for item 'value_type': // Typedefs nested in classes are not supported yet // google3/nowhere/llvm/toolchain/include/c++/v1/__type_traits/integral_constant.h;l=25 -// Error while generating bindings for item 'std::integral_constant<bool, true>::type': +// Error while generating bindings for item 'type': // Typedefs nested in classes are not supported yet // <unknown location> -// Error while generating bindings for item 'std::integral_constant<bool, true>::operator bool': -// Return type is not supported: Unsupported type 'std::integral_constant<_Bool, true>::value_type': No generated bindings found for 'value_type' - -// <unknown location> // Error while generating bindings for item 'std::integral_constant<bool, true>::operator()': -// Return type is not supported: Unsupported type 'std::integral_constant<_Bool, true>::value_type': No generated bindings found for 'value_type' +// Bindings for this kind of operator (operator () with 1 parameter(s)) are not supported mod detail { #[allow(unused_imports)]