Add trivial destructor flags to the IR.
Actually we don't need this, but I had it written anyway...
At the very least, we can omit calls to the destructor if it's trivial, and have an empty Drop impl. At the *most*, we can omit the Drop impl entirely, but this would mean that adding a nontrivial destructor can trigger dropck in Rust code, and isn't super compatible -- we need to talk about this separately.
In order to walk source files that even *have* destructors defined in them, without crashing, we actually also need to check that the identifier for the function is representable before converting to an Identifier, so we do that also.
PiperOrigin-RevId: 400951408
diff --git a/rs_bindings_from_cc/ast_visitor.cc b/rs_bindings_from_cc/ast_visitor.cc
index 131f938..009c2a5 100644
--- a/rs_bindings_from_cc/ast_visitor.cc
+++ b/rs_bindings_from_cc/ast_visitor.cc
@@ -54,7 +54,12 @@
// TODO(b/200239975): Add diagnostics for declarations we can't import
return true;
}
- params.push_back({*param_type, GetTranslatedName(param)});
+ std::optional<Identifier> param_name = GetTranslatedName(param);
+ if (!param_name.has_value()) {
+ // TODO(b/200239975): Add diagnostics for declarations we can't import
+ return true;
+ }
+ params.push_back({*param_type, *std::move(param_name)});
}
auto return_type = ConvertType(function_decl->getReturnType(),
@@ -62,8 +67,13 @@
if (!return_type.ok()) {
return true;
}
+ std::optional<Identifier> translated_name = GetTranslatedName(function_decl);
+ if (!translated_name.has_value()) {
+ // For example, the destructor.
+ return true;
+ }
ir_.items.push_back(Func{
- .identifier = GetTranslatedName(function_decl),
+ .identifier = *translated_name,
.mangled_name = GetMangledName(function_decl),
.return_type = *return_type,
.params = std::move(params),
@@ -102,6 +112,10 @@
.definition = SpecialMemberFunc::Definition::kTrivial,
.access = kPublic,
};
+ SpecialMemberFunc dtor = {
+ .definition = SpecialMemberFunc::Definition::kTrivial,
+ .access = kPublic,
+ };
if (const auto* cxx_record_decl =
clang::dyn_cast<clang::CXXRecordDecl>(record_decl)) {
if (cxx_record_decl->isClass()) {
@@ -128,6 +142,12 @@
move_ctor.definition = SpecialMemberFunc::Definition::kDeleted;
}
+ if (cxx_record_decl->hasTrivialDestructor()) {
+ dtor.definition = SpecialMemberFunc::Definition::kTrivial;
+ } else {
+ dtor.definition = SpecialMemberFunc::Definition::kNontrivial;
+ }
+
for (clang::CXXConstructorDecl* ctor_decl : cxx_record_decl->ctors()) {
if (ctor_decl->isCopyConstructor()) {
copy_ctor.access = TranslateAccessSpecifier(ctor_decl->getAccess());
@@ -141,6 +161,13 @@
}
}
}
+ clang::CXXDestructorDecl* dtor_decl = cxx_record_decl->getDestructor();
+ if (dtor_decl != nullptr) {
+ dtor.access = TranslateAccessSpecifier(dtor_decl->getAccess());
+ if (dtor_decl->isDeleted()) {
+ dtor.definition = SpecialMemberFunc::Definition::kDeleted;
+ }
+ }
}
const clang::ASTRecordLayout& layout =
record_decl->getASTContext().getASTRecordLayout(record_decl);
@@ -154,19 +181,29 @@
if (access == clang::AS_none) {
access = default_access;
}
+
+ std::optional<Identifier> field_name = GetTranslatedName(field_decl);
+ if (!field_name.has_value()) {
+ return true;
+ }
fields.push_back(
- {.identifier = GetTranslatedName(field_decl),
+ {.identifier = *std::move(field_name),
.type = *type,
.access = TranslateAccessSpecifier(access),
.offset = layout.getFieldOffset(field_decl->getFieldIndex())});
}
+ std::optional<Identifier> record_name = GetTranslatedName(record_decl);
+ if (!record_name.has_value()) {
+ return true;
+ }
ir_.items.push_back(
- Record{.identifier = GetTranslatedName(record_decl),
+ Record{.identifier = *record_name,
.fields = std::move(fields),
.size = layout.getSize().getQuantity(),
.alignment = layout.getAlignment().getQuantity(),
.copy_constructor = copy_ctor,
.move_constructor = move_ctor,
+ .destructor = dtor,
.is_trivial_abi = record_decl->canPassInRegisters()});
return true;
}
@@ -240,9 +277,13 @@
return name;
}
-Identifier AstVisitor::GetTranslatedName(
+std::optional<Identifier> AstVisitor::GetTranslatedName(
const clang::NamedDecl* named_decl) const {
- return Identifier(std::string(named_decl->getName()));
+ clang::IdentifierInfo* id = named_decl->getIdentifier();
+ if (id == nullptr) {
+ return std::nullopt;
+ }
+ return Identifier(std::string(id->getName()));
}
} // namespace rs_bindings_from_cc
diff --git a/rs_bindings_from_cc/ast_visitor.h b/rs_bindings_from_cc/ast_visitor.h
index b7a1fba..380205a 100644
--- a/rs_bindings_from_cc/ast_visitor.h
+++ b/rs_bindings_from_cc/ast_visitor.h
@@ -43,7 +43,11 @@
private:
std::string GetMangledName(const clang::NamedDecl* named_decl) const;
- Identifier GetTranslatedName(const clang::NamedDecl* named_decl) const;
+ // Gets the identifier naming the symbol.
+ // Returns nullopt for things with non-identifier names, such as the
+ // destructor.
+ std::optional<Identifier> GetTranslatedName(
+ const clang::NamedDecl* named_decl) const;
absl::StatusOr<MappedType> ConvertType(clang::QualType qual_type,
const clang::ASTContext& ctx) const;
diff --git a/rs_bindings_from_cc/ast_visitor_test.cc b/rs_bindings_from_cc/ast_visitor_test.cc
index 735fd17..3c521cb 100644
--- a/rs_bindings_from_cc/ast_visitor_test.cc
+++ b/rs_bindings_from_cc/ast_visitor_test.cc
@@ -168,6 +168,12 @@
AllOf(matchers...));
}
+// Matches a Record with a destructor that matches all given matchers.
+template <typename... Args>
+auto Destructor(const Args&... matchers) {
+ return testing::Field("destructor", &Record::destructor, AllOf(matchers...));
+}
+
// Matches a SpecialMemberFunc that has the given definition.
MATCHER_P(DefinitionIs, definition, "") { return arg.definition == definition; }
@@ -392,6 +398,61 @@
Each(VariantWith<Record>(MoveConstructor(AccessIs(kPrivate)))));
}
+TEST(AstVisitorTest, TrivialDestructor) {
+ absl::string_view file =
+ "struct Implicit {};\n"
+ "struct Defaulted { ~Defaulted() = default; };\n";
+ IR ir = IrFromCc({file}, {});
+
+ EXPECT_THAT(ir.items, SizeIs(2));
+ EXPECT_THAT(ir.items, Each(VariantWith<Record>(Destructor(DefinitionIs(
+ SpecialMemberFunc::Definition::kTrivial)))));
+}
+
+TEST(AstVisitorTest, NontrivialDestructor) {
+ absl::string_view file = "struct Defined { ~Defined();};\n";
+ // TODO(b/202113881): "struct MemberImplicit { Defined x; };\n"
+ // TODO(b/202113881): "struct MemberDefaulted { ~MemberDefaulted() = default;
+ // Defined x; };\n"
+ IR ir = IrFromCc({file}, {});
+ EXPECT_THAT(ir.items, SizeIs(1));
+ EXPECT_THAT(ir.items, Each(VariantWith<Record>(Destructor(DefinitionIs(
+ SpecialMemberFunc::Definition::kNontrivial)))));
+}
+
+TEST(AstVisitorTest, DeletedDestructor) {
+ absl::string_view file = "struct Deleted { ~Deleted() = delete;};\n";
+ // TODO(b/202113881): "struct DeletedByMember { Deleted x; };\n"
+ IR ir = IrFromCc({file}, {});
+
+ EXPECT_THAT(ir.items, SizeIs(1));
+ EXPECT_THAT(ir.items, Each(VariantWith<Record>(Destructor(DefinitionIs(
+ SpecialMemberFunc::Definition::kDeleted)))));
+}
+
+TEST(AstVisitorTest, PublicDestructor) {
+ absl::string_view file =
+ "class Implicit {};\n"
+ "struct Defaulted { ~Defaulted() = default; };\n"
+ "class Section { public: ~Section() = default; };\n";
+ IR ir = IrFromCc({file}, {});
+
+ EXPECT_THAT(ir.items, SizeIs(3));
+ EXPECT_THAT(ir.items,
+ Each(VariantWith<Record>(Destructor(AccessIs(kPublic)))));
+}
+
+TEST(AstVisitorTest, PrivateDestructor) {
+ absl::string_view file =
+ "class Defaulted { ~Defaulted() = default; };\n"
+ "struct Section { private: ~Section() = default; };\n";
+ IR ir = IrFromCc({file}, {});
+
+ EXPECT_THAT(ir.items, SizeIs(2));
+ EXPECT_THAT(ir.items,
+ Each(VariantWith<Record>(Destructor(AccessIs(kPrivate)))));
+}
+
TEST(AstVisitorTest, MemberVariableAccessSpecifiers) {
IR ir = IrFromCc({R"(
struct SomeStruct {
diff --git a/rs_bindings_from_cc/ir.cc b/rs_bindings_from_cc/ir.cc
index 51a3e75..6145dec 100644
--- a/rs_bindings_from_cc/ir.cc
+++ b/rs_bindings_from_cc/ir.cc
@@ -148,6 +148,7 @@
record["alignment"] = alignment;
record["copy_constructor"] = copy_constructor.ToJson();
record["move_constructor"] = move_constructor.ToJson();
+ record["destructor"] = destructor.ToJson();
record["is_trivial_abi"] = is_trivial_abi;
nlohmann::json item;
diff --git a/rs_bindings_from_cc/ir.h b/rs_bindings_from_cc/ir.h
index 9654b39..848bc3c 100644
--- a/rs_bindings_from_cc/ir.h
+++ b/rs_bindings_from_cc/ir.h
@@ -242,6 +242,7 @@
// Special member functions.
SpecialMemberFunc copy_constructor = {};
SpecialMemberFunc move_constructor = {};
+ SpecialMemberFunc destructor = {};
// Whether this type is passed by value as if it were a trivial type (the same
// as it would be if it were a struct in C).
diff --git a/rs_bindings_from_cc/ir_test.cc b/rs_bindings_from_cc/ir_test.cc
index 991edfe..96b1395 100644
--- a/rs_bindings_from_cc/ir_test.cc
+++ b/rs_bindings_from_cc/ir_test.cc
@@ -116,6 +116,10 @@
"definition": "Deleted",
"access": "Protected"
},
+ "destructor": {
+ "definition": "Trivial",
+ "access": "Public"
+ },
"is_trivial_abi": true
}}
]
@@ -162,6 +166,11 @@
.definition =
SpecialMemberFunc::Definition::kDeleted,
.access = kProtected},
+ .destructor =
+ SpecialMemberFunc{
+ .definition =
+ SpecialMemberFunc::Definition::kTrivial,
+ .access = kPublic},
.is_trivial_abi = true}}};
EXPECT_EQ(ir.ToJson(), expected);
}
