Rename DeclId to ItemId and make all our items have an item id. Namespaces will contain an List<ItemId> that we'll use to generate the right items to the right place. For this, Comments and UnsupportedItems also need an id. For comments we use the address of the RawComment as an id, and the UnsupportedItem gets the id of its own decl (as of https://github.com/google/crubit/commit/593559136eb262fe3870c9827a00f232439a7587 we are generating one pre-IR item per decl, so each UnsupportedItem gets a unique id) PiperOrigin-RevId: 436748182
diff --git a/rs_bindings_from_cc/importer.cc b/rs_bindings_from_cc/importer.cc index 0de6f11..4f35b30 100644 --- a/rs_bindings_from_cc/importer.cc +++ b/rs_bindings_from_cc/importer.cc
@@ -103,8 +103,12 @@ return it->second; } -DeclId GenerateDeclId(const clang::Decl* decl) { - return DeclId(reinterpret_cast<uintptr_t>(decl->getCanonicalDecl())); +ItemId GenerateItemId(const clang::Decl* decl) { + return ItemId(reinterpret_cast<uintptr_t>(decl->getCanonicalDecl())); +} + +ItemId GenerateItemId(const clang::RawComment* comment) { + return ItemId(reinterpret_cast<uintptr_t>(comment)); } std::vector<BaseClass> GetUnambiguousPublicBases( @@ -172,7 +176,7 @@ CRUBIT_CHECK((!offset.hasValue() || *offset >= 0) && "Concrete base classes should have non-negative offsets."); bases.push_back( - BaseClass{.base_record_id = GenerateDeclId(base_record_decl), + BaseClass{.base_record_id = GenerateItemId(base_record_decl), .offset = offset}); break; } @@ -398,7 +402,8 @@ for (auto comment : ImportFreeComments()) { items.push_back(std::make_tuple( comment->getSourceRange(), 0 /* decl_order */, - Comment{.text = comment->getFormattedText(sm, sm.getDiagnostics())})); + Comment{.text = comment->getFormattedText(sm, sm.getDiagnostics()), + .id = GenerateItemId(comment)})); } std::sort(items.begin(), items.end(), is_less_than); @@ -637,7 +642,7 @@ } member_func_metadata = MemberFuncMetadata{ - .record_id = GenerateDeclId(method_decl->getParent()), + .record_id = GenerateItemId(method_decl->getParent()), .instance_method_metadata = instance_metadata}; } @@ -784,7 +789,7 @@ return Record{ .rs_name = std::string(record_name->Ident()), .cc_name = std::string(record_name->Ident()), - .id = GenerateDeclId(record_decl), + .id = GenerateItemId(record_decl), .owning_target = GetOwningTarget(record_decl), .doc_comment = GetComment(record_decl), .unambiguous_public_bases = GetUnambiguousPublicBases(*record_decl, ctx_), @@ -848,7 +853,7 @@ return Enum{ .identifier = *enum_name, - .id = GenerateDeclId(enum_decl), + .id = GenerateItemId(enum_decl), .owning_target = GetOwningTarget(enum_decl), .underlying_type = *std::move(type), .enumerators = enumerators, @@ -862,8 +867,10 @@ name = named_decl->getQualifiedNameAsString(); } SourceLoc source_loc = ConvertSourceLocation(decl->getBeginLoc()); - return UnsupportedItem{ - .name = name, .message = error, .source_loc = source_loc}; + return UnsupportedItem{.name = name, + .message = error, + .source_loc = source_loc, + .id = GenerateItemId(decl)}; } IR::Item Importer::ImportUnsupportedItem(const clang::Decl* decl, @@ -900,7 +907,7 @@ if (underlying_type.ok()) { known_type_decls_.insert(typedef_name_decl); return TypeAlias{.identifier = *identifier, - .id = GenerateDeclId(typedef_name_decl), + .id = GenerateItemId(typedef_name_decl), .owning_target = GetOwningTarget(typedef_name_decl), .doc_comment = GetComment(typedef_name_decl), .underlying_type = *underlying_type}; @@ -961,7 +968,7 @@ "No generated bindings found for '$0'", decl->getNameAsString())); } - DeclId decl_id = GenerateDeclId(decl); + ItemId decl_id = GenerateItemId(decl); return MappedType::WithDeclId(decl_id); }
diff --git a/rs_bindings_from_cc/importer_test.cc b/rs_bindings_from_cc/importer_test.cc index 9fba4d9..d564076 100644 --- a/rs_bindings_from_cc/importer_test.cc +++ b/rs_bindings_from_cc/importer_test.cc
@@ -33,7 +33,7 @@ using ::testing::VariantWith; using ::testing::status::StatusIs; -std::optional<DeclId> DeclIdForRecord(const IR& ir, absl::string_view rs_name) { +std::optional<ItemId> DeclIdForRecord(const IR& ir, absl::string_view rs_name) { for (const Record* record : ir.get_items_if<Record>()) { if (record->rs_name == rs_name) { return record->id; @@ -367,7 +367,7 @@ ASSERT_OK_AND_ASSIGN(IR ir, IrFromCc("struct S{}; const S* Foo(const S* s);")); - std::optional<DeclId> decl_id = DeclIdForRecord(ir, "S"); + std::optional<ItemId> decl_id = DeclIdForRecord(ir, "S"); ASSERT_TRUE(decl_id.has_value()); auto is_ptr_to_const_s =
diff --git a/rs_bindings_from_cc/ir.cc b/rs_bindings_from_cc/ir.cc index 640fb4d..8b22e49 100644 --- a/rs_bindings_from_cc/ir.cc +++ b/rs_bindings_from_cc/ir.cc
@@ -273,6 +273,7 @@ {"member_func_metadata", member_func_metadata}, {"has_c_calling_convention", has_c_calling_convention}, {"source_loc", source_loc}, + {"id", id}, }; return llvm::json::Object{ @@ -409,6 +410,7 @@ {"name", name}, {"message", message}, {"source_loc", source_loc}, + {"id", id}, }; return llvm::json::Object{ @@ -419,6 +421,7 @@ llvm::json::Value Comment::ToJson() const { llvm::json::Object comment{ {"text", text}, + {"id", id}, }; return llvm::json::Object{
diff --git a/rs_bindings_from_cc/ir.h b/rs_bindings_from_cc/ir.h index 58f4d89..8a3b165 100644 --- a/rs_bindings_from_cc/ir.h +++ b/rs_bindings_from_cc/ir.h
@@ -72,11 +72,12 @@ return o << std::string(llvm::formatv("{0:2}", h.ToJson())); } -// An int uniquely representing a Decl. Since our IR goes through the JSON +// An int uniquely representing an Item. Since our IR goes through the JSON // serialization/deserialization at the moment, we need a way to restore graph // edges that don't follow the JSON tree structure (for example between types -// and records). We use DeclIds for this. -CRUBIT_DEFINE_STRONG_INT_TYPE(DeclId, uintptr_t); +// and records), as well as location of comments and items we don't yet support. +// We use ItemIds for this. +CRUBIT_DEFINE_STRONG_INT_TYPE(ItemId, uintptr_t); // A numerical ID that uniquely identifies a lifetime. CRUBIT_DEFINE_STRONG_INT_TYPE(LifetimeId, int); @@ -117,7 +118,7 @@ // Id of a decl that this type corresponds to. `nullopt` when `name` is // non-empty. `llvm::Optional` is used because it integrates better with // `llvm::json` library than `std::optional`. - llvm::Optional<DeclId> decl_id; + llvm::Optional<ItemId> decl_id; // The C++ const-qualification for the type. // @@ -156,7 +157,7 @@ // Id of a decl that this type corresponds to. `nullopt` when `name` is // non-empty. `llvm::Optional` is used because it integrates better with // `llvm::json` library than `std::optional`. - llvm::Optional<DeclId> decl_id; + llvm::Optional<ItemId> decl_id; // Lifetime arguments for a generic type. Examples: // *mut i32 has no lifetime arguments @@ -194,7 +195,7 @@ return MappedType{RsType{rs_name}, CcType{cc_name}}; } - static MappedType WithDeclId(DeclId decl_id) { + static MappedType WithDeclId(ItemId decl_id) { return MappedType{RsType{.decl_id = decl_id}, CcType{.decl_id = decl_id}}; } @@ -362,7 +363,7 @@ llvm::json::Value ToJson() const; // The type that this is a member function for. - DeclId record_id; + ItemId record_id; // Qualifiers for the instance method. // @@ -402,6 +403,7 @@ llvm::Optional<MemberFuncMetadata> member_func_metadata; bool has_c_calling_convention = true; SourceLoc source_loc; + ItemId id; }; inline std::ostream& operator<<(std::ostream& o, const Func& f) { @@ -474,7 +476,7 @@ // A base class subobject of a struct or class. struct BaseClass { llvm::json::Value ToJson() const; - DeclId base_record_id; + ItemId base_record_id; // The offset the base class subobject is located at. This is always nonempty // for nonvirtual inheritance, and always empty if a virtual base class is @@ -495,7 +497,7 @@ std::string rs_name; std::string cc_name; - DeclId id; + ItemId id; BlazeLabel owning_target; llvm::Optional<std::string> doc_comment; std::vector<BaseClass> unambiguous_public_bases; @@ -556,7 +558,7 @@ llvm::json::Value ToJson() const; Identifier identifier; - DeclId id; + ItemId id; BlazeLabel owning_target; MappedType underlying_type; std::vector<Enumerator> enumerators; @@ -571,7 +573,7 @@ llvm::json::Value ToJson() const; Identifier identifier; - DeclId id; + ItemId id; BlazeLabel owning_target; llvm::Optional<std::string> doc_comment; MappedType underlying_type; @@ -595,6 +597,7 @@ // TODO(forster): We should support multiple reasons per unsupported item. std::string message; SourceLoc source_loc; + ItemId id; }; inline std::ostream& operator<<(std::ostream& o, const UnsupportedItem& r) { @@ -605,6 +608,7 @@ llvm::json::Value ToJson() const; std::string text; + ItemId id; }; inline std::ostream& operator<<(std::ostream& o, const Comment& r) {
diff --git a/rs_bindings_from_cc/ir.rs b/rs_bindings_from_cc/ir.rs index 50004a0..6c74ad0 100644 --- a/rs_bindings_from_cc/ir.rs +++ b/rs_bindings_from_cc/ir.rs
@@ -51,13 +51,13 @@ } } } - let decl_id_to_item_idx = flat_ir + let item_id_to_item_idx = flat_ir .items .iter() .enumerate() .filter_map(|(idx, item)| item.id().map(|id| (id, idx))) .collect::<HashMap<_, _>>(); - Ok(IR { flat_ir, decl_id_to_item_idx }) + Ok(IR { flat_ir, item_id_to_item_idx }) } #[derive(Debug, PartialEq, Eq, Hash, Clone, Deserialize)] @@ -80,7 +80,7 @@ pub name: Option<String>, pub lifetime_args: Vec<LifetimeId>, pub type_args: Vec<RsType>, - pub decl_id: Option<DeclId>, + pub decl_id: Option<ItemId>, } impl RsType { @@ -94,7 +94,7 @@ pub name: Option<String>, pub is_const: bool, pub type_args: Vec<CcType>, - pub decl_id: Option<DeclId>, + pub decl_id: Option<ItemId>, } impl CcType { @@ -104,17 +104,17 @@ } pub trait TypeWithDeclId { - fn decl_id(&self) -> Option<DeclId>; + fn decl_id(&self) -> Option<ItemId>; } impl TypeWithDeclId for RsType { - fn decl_id(&self) -> Option<DeclId> { + fn decl_id(&self) -> Option<ItemId> { self.decl_id } } impl TypeWithDeclId for CcType { - fn decl_id(&self) -> Option<DeclId> { + fn decl_id(&self) -> Option<ItemId> { self.decl_id } } @@ -175,7 +175,7 @@ #[derive(Debug, PartialEq, Eq, Hash, Clone, Copy, Deserialize)] #[serde(transparent)] -pub struct DeclId(pub usize); +pub struct ItemId(pub usize); #[derive(Debug, PartialEq, Eq, Hash, Clone, Deserialize)] #[serde(transparent)] @@ -243,7 +243,7 @@ #[derive(Debug, PartialEq, Eq, Hash, Clone, Deserialize)] pub struct MemberFuncMetadata { - pub record_id: DeclId, + pub record_id: ItemId, pub instance_method_metadata: Option<InstanceMethodMetadata>, } @@ -273,6 +273,7 @@ pub member_func_metadata: Option<MemberFuncMetadata>, pub has_c_calling_convention: bool, pub source_loc: SourceLoc, + pub id: ItemId, } impl Func { @@ -312,7 +313,7 @@ #[derive(Debug, PartialEq, Eq, Hash, Clone, Deserialize)] pub struct BaseClass { - pub base_record_id: DeclId, + pub base_record_id: ItemId, pub offset: Option<i64>, } @@ -326,7 +327,7 @@ pub struct Record { pub rs_name: String, pub cc_name: String, - pub id: DeclId, + pub id: ItemId, pub owning_target: BlazeLabel, pub doc_comment: Option<String>, pub unambiguous_public_bases: Vec<BaseClass>, @@ -381,7 +382,7 @@ #[derive(Debug, PartialEq, Eq, Hash, Clone, Deserialize)] pub struct Enum { pub identifier: Identifier, - pub id: DeclId, + pub id: ItemId, pub owning_target: BlazeLabel, pub underlying_type: MappedType, pub enumerators: Vec<Enumerator>, @@ -396,7 +397,7 @@ #[derive(Debug, PartialEq, Eq, Hash, Clone, Deserialize)] pub struct TypeAlias { pub identifier: Identifier, - pub id: DeclId, + pub id: ItemId, pub owning_target: BlazeLabel, pub doc_comment: Option<String>, pub underlying_type: MappedType, @@ -414,11 +415,13 @@ pub name: String, pub message: String, pub source_loc: SourceLoc, + pub id: ItemId, } #[derive(Debug, PartialEq, Eq, Hash, Clone, Deserialize)] pub struct Comment { pub text: String, + pub id: ItemId, } #[derive(Debug, PartialEq, Eq, Hash, Clone, Deserialize)] @@ -432,7 +435,7 @@ } impl Item { - fn id(&self) -> Option<DeclId> { + fn id(&self) -> Option<ItemId> { match self { Item::Record(record) => Some(record.id), Item::TypeAlias(type_alias) => Some(type_alias.id), @@ -522,7 +525,7 @@ pub struct IR { flat_ir: FlatIR, // A map from a `decl_id` to an index of an `Item` in the `flat_ir.items` vec. - decl_id_to_item_idx: HashMap<DeclId, usize>, + item_id_to_item_idx: HashMap<ItemId, usize>, } impl IR { @@ -563,6 +566,13 @@ }) } + pub fn comments(&self) -> impl Iterator<Item = &Comment> { + self.items().filter_map(|item| match item { + Item::Comment(comment) => Some(comment), + _ => None, + }) + } + pub fn item_for_type<T>(&self, ty: &T) -> Result<&Item> where T: TypeWithDeclId + std::fmt::Debug, @@ -575,7 +585,7 @@ } } - pub fn find_decl<'a, T>(&'a self, decl_id: DeclId) -> Result<&'a T> + pub fn find_decl<'a, T>(&'a self, decl_id: ItemId) -> Result<&'a T> where &'a T: TryFrom<&'a Item>, { @@ -586,9 +596,9 @@ }) } - fn find_untyped_decl(&self, decl_id: DeclId) -> Result<&Item> { + fn find_untyped_decl(&self, decl_id: ItemId) -> Result<&Item> { let idx = *self - .decl_id_to_item_idx + .item_id_to_item_idx .get(&decl_id) .with_context(|| format!("Couldn't find decl_id {:?} in the IR.", decl_id))?; self.flat_ir.items.get(idx).with_context(|| format!("Couldn't find an item at idx {}", idx))
diff --git a/rs_bindings_from_cc/ir_from_cc_test.rs b/rs_bindings_from_cc/ir_from_cc_test.rs index 1abae69..af7e193 100644 --- a/rs_bindings_from_cc/ir_from_cc_test.rs +++ b/rs_bindings_from_cc/ir_from_cc_test.rs
@@ -36,6 +36,12 @@ (Item::Enum(ref mut expected_enum), Item::Enum(actual_enum)) => { expected_enum.id = actual_enum.id; } + (Item::Func(ref mut expected_func), Item::Func(actual_func)) => { + expected_func.id = actual_func.id; + } + (Item::TypeAlias(ref mut expected_alias), Item::TypeAlias(actual_alias)) => { + expected_alias.id = actual_alias.id; + } (_, _) => (), } } @@ -63,6 +69,7 @@ line: 3, column: 1, }, + id: ItemId(0), })], ); } @@ -87,6 +94,7 @@ line: 3, column: 1, }, + id: ItemId(0), })], ); } @@ -642,7 +650,7 @@ quote! { TypeAlias { identifier: "MyTypedefDecl", - id: DeclId(...), + id: ItemId(...), owning_target: BlazeLabel("//test:testing_target"), doc_comment: Some("Doc comment for MyTypedefDecl."), underlying_type: #int, @@ -654,7 +662,7 @@ quote! { TypeAlias { identifier: "MyTypeAliasDecl", - id: DeclId(...), + id: ItemId(...), owning_target: BlazeLabel("//test:testing_target"), doc_comment: Some("Doc comment for MyTypeAliasDecl."), underlying_type: #int, @@ -1347,3 +1355,17 @@ } ); } + +#[test] +fn test_unsupported_item_has_item_id() { + let ir = ir_from_cc("union SomeUnion {};").unwrap(); + let unsupported = ir.unsupported_items().find(|i| i.name == "SomeUnion").unwrap(); + assert_ne!(unsupported.id, ItemId(0)); +} + +#[test] +fn test_comment_has_item_id() { + let ir = ir_from_cc("// Comment").unwrap(); + let comment = ir.comments().find(|i| i.text == "Comment").unwrap(); + assert_ne!(comment.id, ItemId(0)); +}
diff --git a/rs_bindings_from_cc/ir_testing.rs b/rs_bindings_from_cc/ir_testing.rs index 8f10ba7..ce39b6f 100644 --- a/rs_bindings_from_cc/ir_testing.rs +++ b/rs_bindings_from_cc/ir_testing.rs
@@ -6,7 +6,7 @@ use ffi_types::{FfiU8Slice, FfiU8SliceBox}; use ir::{ - self, CcType, DeclId, Func, FuncParam, Identifier, Item, MappedType, Record, RsType, + self, CcType, Func, FuncParam, Identifier, Item, ItemId, MappedType, Record, RsType, SpecialMemberFunc, IR, }; @@ -73,13 +73,13 @@ name: None, lifetime_args: vec![], type_args: vec![], - decl_id: Some(DeclId(decl_id)), + decl_id: Some(ItemId(decl_id)), }, cc_type: CcType { name: None, type_args: vec![], is_const: false, - decl_id: Some(DeclId(decl_id)), + decl_id: Some(ItemId(decl_id)), }, } }
diff --git a/rs_bindings_from_cc/src_code_gen.rs b/rs_bindings_from_cc/src_code_gen.rs index ab311d4..9f616d4 100644 --- a/rs_bindings_from_cc/src_code_gen.rs +++ b/rs_bindings_from_cc/src_code_gen.rs
@@ -215,6 +215,7 @@ name: cxx_function_name(func, ir)?, message: message.to_string(), source_loc: func.source_loc.clone(), + id: func.id, }) } @@ -1257,7 +1258,7 @@ // TODO(b/213947473): Instead of having a separate RsTypeKind here, consider // changing ir::RsType into a similar `enum`, with fields that contain -// references (e.g. &'ir Record`) instead of DeclIds. +// references (e.g. &'ir Record`) instead of ItemIds. #[derive(Debug)] enum RsTypeKind<'ir> { Pointer { pointee: Box<RsTypeKind<'ir>>, mutability: Mutability }, @@ -1993,9 +1994,9 @@ // `ir_testing`. fn test_duplicate_decl_ids_err() { let mut r1 = ir_record("R1"); - r1.id = DeclId(42); + r1.id = ItemId(42); let mut r2 = ir_record("R2"); - r2.id = DeclId(42); + r2.id = ItemId(42); let result = make_ir_from_items([r1.into(), r2.into()]); assert!(result.is_err()); assert!(result.unwrap_err().to_string().contains("Duplicate decl_id found in"));