Fields of unsupported type can be represented as opaque blobs of bytes.
After this CL the IR::Field::type value is wrapped in absl::StatusOr (or
Result<_, String> on Rust side) - an error means an unsupported type.
PiperOrigin-RevId: 449355244
diff --git a/rs_bindings_from_cc/BUILD b/rs_bindings_from_cc/BUILD
index db38b32..293ae6b 100644
--- a/rs_bindings_from_cc/BUILD
+++ b/rs_bindings_from_cc/BUILD
@@ -251,6 +251,7 @@
"//common:check",
"//common:strong_int",
"@absl//container:flat_hash_map",
+ "@absl//status:statusor",
"@absl//strings",
"@llvm///clang:ast",
"@llvm///llvm:Support",
diff --git a/rs_bindings_from_cc/importers/cxx_record.cc b/rs_bindings_from_cc/importers/cxx_record.cc
index 0ddb5b9..029e4c1 100644
--- a/rs_bindings_from_cc/importers/cxx_record.cc
+++ b/rs_bindings_from_cc/importers/cxx_record.cc
@@ -97,7 +97,7 @@
}
for (const Field& field : *fields) {
- if (field.is_no_unique_address) {
+ if (field.is_no_unique_address || !field.type.ok()) {
override_alignment = true;
break;
}
@@ -144,13 +144,9 @@
ictx_.ctx_.getASTRecordLayout(record_decl);
for (const clang::FieldDecl* field_decl : record_decl->fields()) {
std::optional<clang::tidy::lifetimes::ValueLifetimes> no_lifetimes;
- auto type =
+ absl::StatusOr<MappedType> type =
temp_import_mapper.ConvertQualType(field_decl->getType(), no_lifetimes);
- if (!type.ok()) {
- return absl::UnimplementedError(absl::Substitute(
- "Type of field '$0' is not supported: $1",
- field_decl->getNameAsString(), type.status().message()));
- }
+
clang::AccessSpecifier access = field_decl->getAccess();
if (access == clang::AS_none) {
access = default_access;
@@ -166,7 +162,7 @@
{.identifier = field_name ? *std::move(field_name)
: llvm::Optional<Identifier>(llvm::None),
.doc_comment = ictx_.GetComment(field_decl),
- .type = *type,
+ .type = std::move(type),
.access = TranslateAccessSpecifier(access),
.offset = layout.getFieldOffset(field_decl->getFieldIndex()),
.size = ictx_.ctx_.getTypeSize(field_decl->getType()),
diff --git a/rs_bindings_from_cc/importers/cxx_record.h b/rs_bindings_from_cc/importers/cxx_record.h
index 5ffafb2..f9c8012 100644
--- a/rs_bindings_from_cc/importers/cxx_record.h
+++ b/rs_bindings_from_cc/importers/cxx_record.h
@@ -15,6 +15,8 @@
std::optional<IR::Item> Import(clang::CXXRecordDecl*);
private:
+ // TODO(b/226580208): absl::StatusOr is obsolete below / ImportFields cannot
+ // fail.
absl::StatusOr<std::vector<Field>> ImportFields(clang::CXXRecordDecl*);
std::vector<BaseClass> GetUnambiguousPublicBases(
const clang::CXXRecordDecl& record_decl) const;
diff --git a/rs_bindings_from_cc/ir.cc b/rs_bindings_from_cc/ir.cc
index 5e2f533..4495826 100644
--- a/rs_bindings_from_cc/ir.cc
+++ b/rs_bindings_from_cc/ir.cc
@@ -36,6 +36,14 @@
return llvm::json::Value(string_type.value());
}
+template <class T>
+llvm::json::Value toJSON(const absl::StatusOr<T>& t) {
+ if (t.ok()) {
+ return llvm::json::Object{{"Ok", *t}};
+ }
+ return llvm::json::Object{{"Err", std::string(t.status().message())}};
+}
+
llvm::json::Value HeaderName::ToJson() const {
return llvm::json::Object{
{"name", name_},
diff --git a/rs_bindings_from_cc/ir.h b/rs_bindings_from_cc/ir.h
index 9c5431b..d6f3394 100644
--- a/rs_bindings_from_cc/ir.h
+++ b/rs_bindings_from_cc/ir.h
@@ -22,6 +22,7 @@
#include <vector>
#include "absl/container/flat_hash_map.h"
+#include "absl/status/statusor.h"
#include "absl/strings/string_view.h"
#include "common/check.h"
#include "common/strong_int.h"
@@ -479,7 +480,7 @@
llvm::Optional<Identifier> identifier;
llvm::Optional<std::string> doc_comment;
- MappedType type;
+ absl::StatusOr<MappedType> type;
AccessSpecifier access;
// Field offset in bits.
uint64_t offset;
diff --git a/rs_bindings_from_cc/ir.rs b/rs_bindings_from_cc/ir.rs
index 5b407bb..7821c3a 100644
--- a/rs_bindings_from_cc/ir.rs
+++ b/rs_bindings_from_cc/ir.rs
@@ -309,7 +309,7 @@
pub identifier: Option<Identifier>,
pub doc_comment: Option<String>,
#[serde(rename(deserialize = "type"))]
- pub type_: MappedType,
+ pub type_: Result<MappedType, String>,
pub access: AccessSpecifier,
pub offset: usize,
pub size: usize,
diff --git a/rs_bindings_from_cc/ir_from_cc_test.rs b/rs_bindings_from_cc/ir_from_cc_test.rs
index 9fc2817..10f1aec 100644
--- a/rs_bindings_from_cc/ir_from_cc_test.rs
+++ b/rs_bindings_from_cc/ir_from_cc_test.rs
@@ -359,7 +359,7 @@
quote! {
Field {
identifier: Some("ptr") ...
- type_: MappedType {
+ type_: Ok(MappedType {
rs_type: RsType {
name: Some("*mut") ...
type_args: [RsType {
@@ -378,7 +378,7 @@
}],
decl_id: None,
},
- } ...
+ }) ...
}
}
);
@@ -576,9 +576,10 @@
let fields = ir.records().next().unwrap().fields.iter();
let type_mapping: HashMap<_, _> = fields
.map(|f| {
+
(
- f.type_.cc_type.name.as_ref().unwrap().as_str(),
- f.type_.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();
@@ -779,10 +780,10 @@
fields: [Field {
identifier: Some("value"), ...
doc_comment: Some("Doc comment of `value` field."), ...
- type_: MappedType {
+ type_: Ok(MappedType {
rs_type: RsType { name: Some("i32"), ... },
cc_type: CcType { name: Some("int"), ... },
- },
+ }),
access: Public,
offset: 0, ...
}], ...
@@ -890,10 +891,10 @@
fields: [Field {
identifier: Some("value"), ...
doc_comment: Some("Doc comment of the `value` field specialization for T=int."), ...
- type_: MappedType {
+ type_: Ok(MappedType {
rs_type: RsType { name: Some("i32"), ... },
cc_type: CcType { name: Some("int"), ... },
- },
+ }),
access: Public,
offset: 0, ...
}], ...
@@ -1564,13 +1565,15 @@
}
#[test]
-fn test_record_with_unsupported_field() -> Result<()> {
+fn test_record_with_unsupported_field_type() -> Result<()> {
// Using a nested struct because it's currently not supported.
// But... any other unsupported type would also work for this test.
let ir = ir_from_cc(
r#"
struct StructWithUnsupportedField {
struct NestedStruct {};
+
+ // Doc comment for `my_field`.
NestedStruct my_field;
};
"#,
@@ -1578,11 +1581,36 @@
assert_ir_matches!(
ir,
quote! {
- UnsupportedItem(UnsupportedItem {
- name: "StructWithUnsupportedField",
- message: "UNIMPLEMENTED: Type of field 'my_field' is not supported: Unsupported type 'struct StructWithUnsupportedField::NestedStruct': No generated bindings found for 'NestedStruct'",
+ Record {
+ rs_name: "StructWithUnsupportedField",
+ cc_name: "StructWithUnsupportedField",
+ ...
+ fields: [Field {
+ identifier: Some("my_field"),
+ doc_comment: Some("Doc comment for `my_field`."),
+ type_: Err(
+ "Unsupported type 'struct StructWithUnsupportedField::NestedStruct': No generated bindings found for 'NestedStruct'",
+ ),
+ access: Public,
+ offset: 0,
+ size: 8,
+ is_no_unique_address: false,
+ }],
+ ...
+ size: 1,
+ alignment: 1,
+ ...
+ }
+ }
+ );
+ assert_ir_matches!(
+ ir,
+ quote! {
+ UnsupportedItem {
+ name: "StructWithUnsupportedField::NestedStruct",
+ message: "Nested classes are not supported yet",
...
- })
+ }
}
);
Ok(())
@@ -1756,19 +1784,19 @@
fields: [
Field {
identifier: Some("first_field"), ...
- type_ : MappedType {
+ type_: Ok(MappedType {
rs_type : RsType { name : Some ("i32"), ...},
cc_type : CcType { name : Some ("int"), ...},
- }, ...
+ }), ...
offset: 0, ...
size: 32, ...
},
Field {
identifier: Some("second_field"), ...
- type_ : MappedType {
+ type_: Ok(MappedType {
rs_type : RsType { name : Some ("i32"), ...},
cc_type : CcType { name : Some ("int"), ...},
- }, ...
+ }), ...
offset: 32, ...
size: 32, ...
},
@@ -1799,19 +1827,19 @@
fields: [
Field {
identifier: Some("first_field"), ...
- type_ : MappedType {
+ type_: Ok(MappedType {
rs_type : RsType { name : Some ("i32"), ...},
cc_type : CcType { name : Some ("int"), ...},
- }, ...
+ }), ...
offset: 0, ...
size: 32, ...
},
Field {
identifier: Some("second_field"), ...
- type_ : MappedType {
+ type_: Ok(MappedType {
rs_type : RsType { name : Some ("i32"), ...},
cc_type : CcType { name : Some ("int"), ...},
- }, ...
+ }), ...
offset: 0, ...
size: 32, ...
},
@@ -2057,7 +2085,6 @@
let names = ir.unsupported_items().map(|i| i.name.as_str()).collect_vec();
assert_strings_dont_contain(names.as_slice(), "OuterStruct");
assert_strings_dont_contain(names.as_slice(), "NestedStructIsUnsupported");
- assert_strings_contain(names.as_slice(), "MyOtherStruct");
Ok(())
}
diff --git a/rs_bindings_from_cc/src_code_gen.rs b/rs_bindings_from_cc/src_code_gen.rs
index 05c1659..30418f5 100644
--- a/rs_bindings_from_cc/src_code_gen.rs
+++ b/rs_bindings_from_cc/src_code_gen.rs
@@ -964,14 +964,19 @@
}
}
-fn should_represent_field_as_opaque_blob(field: &Field) -> bool {
- // [[no_unique_address]] fields are replaced by an unaligned block of memory
- // which fills space up to the next field.
+/// Gets the type of `field` for layout purposes.
+///
+/// Note that `get_field_rs_type_for_layout` may return Err (for
+/// `is_no_unique_address` fields) even if `field.type_` is Ok.
+fn get_field_rs_type_for_layout(field: &Field) -> Result<&RsType, &str> {
+ // [[no_unique_address]] fields are replaced by a type-less, unaligned block of
+ // memory which fills space up to the next field.
// See: docs/struct_layout
- //
- // TODO(b/226580208): Fields of unsupported types should also be represented as
- // an opaque blob of bytes.
- field.is_no_unique_address
+ if field.is_no_unique_address {
+ return Err("`[[no_unique_address]]` attribute was present.");
+ }
+
+ field.type_.as_ref().map(|t| &t.rs_type).map_err(String::as_str)
}
/// Generates Rust source code for a given `Record` and associated assertions as
@@ -994,48 +999,61 @@
.iter()
.enumerate()
.map(|(field_index, field)| {
- let is_opaque_blob = should_represent_field_as_opaque_blob(field);
-
let ident = make_rs_field_ident(field, field_index);
- let doc_comment = generate_doc_comment(&field.doc_comment);
- let access = if field.access == AccessSpecifier::Public && !is_opaque_blob {
+ let doc_comment = match field.type_.as_ref() {
+ Ok(_) => generate_doc_comment(&field.doc_comment),
+ Err(msg) => {
+ let supplemental_text =
+ format!("Reason for representing this field as a blob of bytes:\n{}", msg);
+ let new_text = match field.doc_comment.as_ref() {
+ None => supplemental_text,
+ Some(old_text) => format!("{}\n\n{}", old_text, supplemental_text),
+ };
+ generate_doc_comment(&Some(new_text))
+ }
+ };
+ let access = if field.access == AccessSpecifier::Public
+ && get_field_rs_type_for_layout(field).is_ok()
+ {
quote! { pub }
} else {
quote! {}
};
- let field_type = if is_opaque_blob {
- let next_offset = if let Some(next) = record.fields.get(field_index + 1) {
- next.offset
- } else {
- record.size * 8
- };
- let width = Literal::usize_unsuffixed((next_offset - field.offset) / 8);
- quote! {[crate::rust_std::mem::MaybeUninit<u8>; #width]}
- } else {
- let mut formatted =
- format_rs_type(&field.type_.rs_type, ir).with_context(|| {
+ let field_type = match get_field_rs_type_for_layout(field) {
+ Err(_) => {
+ let next_offset = if let Some(next) = record.fields.get(field_index + 1) {
+ next.offset
+ } else {
+ record.size * 8
+ };
+ let width = Literal::usize_unsuffixed((next_offset - field.offset) / 8);
+ quote! {[crate::rust_std::mem::MaybeUninit<u8>; #width]}
+ }
+ Ok(rs_type) => {
+ let mut formatted = format_rs_type(&rs_type, ir).with_context(|| {
format!(
"Failed to format type for field {:?} on record {:?}",
field, record
)
})?;
- if should_implement_drop(record) || record.is_union {
- if needs_manually_drop(&field.type_.rs_type, ir)? {
- // TODO(b/212690698): Avoid (somewhat unergonomic) ManuallyDrop
- // if we can ask Rust to preserve field destruction order if the
- // destructor is the SpecialMemberDefinition::NontrivialMembers
- // case.
- formatted = quote! { crate::rust_std::mem::ManuallyDrop<#formatted> }
- } else {
- field_copy_trait_assertions.push(quote! {
- const _: () = {
- static_assertions::assert_impl_all!(#formatted: Copy);
- };
- });
- }
- };
- formatted
+ if should_implement_drop(record) || record.is_union {
+ if needs_manually_drop(rs_type, ir)? {
+ // TODO(b/212690698): Avoid (somewhat unergonomic) ManuallyDrop
+ // if we can ask Rust to preserve field destruction order if the
+ // destructor is the SpecialMemberDefinition::NontrivialMembers
+ // case.
+ formatted = quote! { crate::rust_std::mem::ManuallyDrop<#formatted> }
+ } else {
+ field_copy_trait_assertions.push(quote! {
+ const _: () = {
+ static_assertions::assert_impl_all!(#formatted: Copy);
+ };
+ });
+ }
+ };
+ formatted
+ }
};
// `is_opaque_blob` representation is always unaligned, even though the actual
@@ -1048,11 +1066,11 @@
};
let padding_size_in_bytes = match prev_field {
None => 0,
- // No padding should be needed if the type of the current field is present
+ // No padding should be needed if the type of the current field is known
// (i.e. if the current field is correctly aligned based on its original type).
- Some(_) if !is_opaque_blob => 0,
+ Some(_) if get_field_rs_type_for_layout(field).is_ok() => 0,
Some(prev_field) => {
- let current_offset = if should_represent_field_as_opaque_blob(prev_field) {
+ let current_offset = if get_field_rs_type_for_layout(prev_field).is_err() {
field.offset
} else {
prev_field.offset + prev_field.size
@@ -2270,16 +2288,20 @@
if field.access != AccessSpecifier::Public || !field.is_no_unique_address {
continue;
}
- fields.push(make_rs_ident(
- &field
- .identifier
- .as_ref()
- .expect("Unnamed fields can't be annotated with [[no_unique_address]]")
- .identifier,
- ));
- types.push(format_rs_type(&field.type_.rs_type, ir).with_context(|| {
- format!("Failed to format type for field {:?} on record {:?}", field, record)
- })?)
+ // Can't use `get_field_rs_type_for_layout` here, because we want to dig into
+ // no_unique_address fields, despite laying them out as opaque blobs of bytes.
+ if let Ok(rs_type) = field.type_.as_ref().map(|t| &t.rs_type) {
+ fields.push(make_rs_ident(
+ &field
+ .identifier
+ .as_ref()
+ .expect("Unnamed fields can't be annotated with [[no_unique_address]]")
+ .identifier,
+ ));
+ types.push(format_rs_type(rs_type, ir).with_context(|| {
+ format!("Failed to format type for field {:?} on record {:?}", field, record)
+ })?)
+ }
}
if fields.is_empty() {
@@ -2929,6 +2951,40 @@
}
#[test]
+ fn test_record_with_unsupported_field_type() -> Result<()> {
+ // Using a nested struct because it's currently not supported.
+ // But... any other unsupported type would also work for this test.
+ let ir = ir_from_cc(
+ r#"
+ struct StructWithUnsupportedField {
+ struct NestedStruct {
+ int nested_field;
+ };
+
+ // Doc comment for `my_field`.
+ NestedStruct my_field;
+ };
+ "#,
+ )?;
+ let rs_api = generate_bindings_tokens(&ir)?.rs_api;
+ assert_rs_matches!(
+ rs_api,
+ quote! {
+ #[repr(C, align(4))]
+ pub struct StructWithUnsupportedField {
+ #[doc = " Doc comment for `my_field`.\n \n Reason for representing this field as a blob of bytes:\n Unsupported type 'struct StructWithUnsupportedField::NestedStruct': No generated bindings found for 'NestedStruct'"]
+ my_field: [crate::rust_std::mem::MaybeUninit<u8>; 4],
+ }
+ ...
+ const _: () = assert!(
+ memoffset_unstable_const::offset_of!(
+ crate::StructWithUnsupportedField, my_field) * 8 == 0);
+ }
+ );
+ Ok(())
+ }
+
+ #[test]
fn test_struct_from_other_target() -> Result<()> {
let ir = ir_from_cc_dependency("// intentionally empty", "struct SomeStruct {};")?;
let BindingsTokens { rs_api, rs_api_impl } = generate_bindings_tokens(&ir)?;
diff --git a/rs_bindings_from_cc/test/golden/unsupported.h b/rs_bindings_from_cc/test/golden/unsupported.h
index fefc50a..c0db936 100644
--- a/rs_bindings_from_cc/test/golden/unsupported.h
+++ b/rs_bindings_from_cc/test/golden/unsupported.h
@@ -32,6 +32,9 @@
void NonStaticMemberFunction();
void StaticMemberFunction();
};
+
+ // Doc comment for an unsupported field.
+ NestedStruct nested_struct;
};
#endif // CRUBIT_RS_BINDINGS_FROM_CC_TEST_GOLDEN_UNSUPPORTED_H_
diff --git a/rs_bindings_from_cc/test/golden/unsupported_rs_api.rs b/rs_bindings_from_cc/test/golden/unsupported_rs_api.rs
index 23e9b5d..6f17fbc 100644
--- a/rs_bindings_from_cc/test/golden/unsupported_rs_api.rs
+++ b/rs_bindings_from_cc/test/golden/unsupported_rs_api.rs
@@ -82,7 +82,11 @@
#[derive(Clone, Copy)]
#[repr(C)]
pub struct ContainingStruct {
- __non_field_data: [crate::rust_std::mem::MaybeUninit<u8>; 1],
+ /// Doc comment for an unsupported field.
+ ///
+ /// Reason for representing this field as a blob of bytes:
+ /// Unsupported type 'struct ContainingStruct::NestedStruct': No generated bindings found for 'NestedStruct'
+ nested_struct: [crate::rust_std::mem::MaybeUninit<u8>; 1],
}
forward_declare::unsafe_define!(
forward_declare::symbol!("ContainingStruct"),
@@ -168,3 +172,5 @@
const _: () = {
static_assertions::assert_not_impl_all!(crate::ContainingStruct: Drop);
};
+const _: () =
+ assert!(memoffset_unstable_const::offset_of!(crate::ContainingStruct, nested_struct) * 8 == 0);
diff --git a/rs_bindings_from_cc/test/golden/unsupported_rs_api_impl.cc b/rs_bindings_from_cc/test/golden/unsupported_rs_api_impl.cc
index de60a7d..6e998bb 100644
--- a/rs_bindings_from_cc/test/golden/unsupported_rs_api_impl.cc
+++ b/rs_bindings_from_cc/test/golden/unsupported_rs_api_impl.cc
@@ -48,5 +48,6 @@
static_assert(sizeof(class ContainingStruct) == 1);
static_assert(alignof(class ContainingStruct) == 1);
+static_assert(CRUBIT_OFFSET_OF(nested_struct, class ContainingStruct) * 8 == 0);
#pragma clang diagnostic pop
