[Lldb-commits] [lldb] [lldb][TypeSystem] Pass ClangASTMetadata by-value when creating record/objc types (PR #102296)
Michael Buch via lldb-commits
lldb-commits at lists.llvm.org
Wed Aug 7 04:50:04 PDT 2024
https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/102296
>From 45c8f9534ec0ddc50c825fa7a3d048746b74360c Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Wed, 7 Aug 2024 11:03:25 +0100
Subject: [PATCH 1/2] [lldb][TypeSystem] Pass ClangASTMetadata by-value when
creating record/objc types
This is a follow-up to https://github.com/llvm/llvm-project/pull/102161
where we changed the `GetMetadata`/`SetMetadata` APIs to pass
`ClangASTMetadata` by-value, instead of `ClangASTMetadata *`, which
wasn't a very friendly API.
This patch continues from there and changes `CreateRecordType`/`CreateObjCClass` to take the metadata by-value as well.
As a drive-by change, I also changed `DelayedAddObjCClassProperty` to
store the metadata by-value, instead of in a `std::unique_ptr`, which
AFAICT, was done solely due to the TypeSystemClang APIs taking the
metadata by pointer.
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 39 ++++---------------
.../SymbolFile/NativePDB/PdbAstBuilder.cpp | 2 +-
.../NativePDB/UdtRecordCompleter.cpp | 2 +-
.../Plugins/SymbolFile/PDB/PDBASTParser.cpp | 2 +-
.../TypeSystem/Clang/TypeSystemClang.cpp | 16 ++++----
.../TypeSystem/Clang/TypeSystemClang.h | 28 ++++++-------
6 files changed, 31 insertions(+), 58 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index a4dcde1629fc2..1a13725b99804 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1803,7 +1803,7 @@ DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc,
if (!clang_type) {
clang_type = m_ast.CreateRecordType(
containing_decl_ctx, GetOwningClangModule(die), attrs.accessibility,
- attrs.name.GetCString(), tag_decl_kind, attrs.class_language, &metadata,
+ attrs.name.GetCString(), tag_decl_kind, attrs.class_language, metadata,
attrs.exports_symbols);
}
@@ -1883,43 +1883,18 @@ class DWARFASTParserClang::DelayedAddObjCClassProperty {
// required if you don't have an
// ivar decl
const char *property_setter_name, const char *property_getter_name,
- uint32_t property_attributes, const ClangASTMetadata *metadata)
+ uint32_t property_attributes, ClangASTMetadata metadata)
: m_class_opaque_type(class_opaque_type), m_property_name(property_name),
m_property_opaque_type(property_opaque_type),
m_property_setter_name(property_setter_name),
m_property_getter_name(property_getter_name),
- m_property_attributes(property_attributes) {
- if (metadata != nullptr) {
- m_metadata_up = std::make_unique<ClangASTMetadata>();
- *m_metadata_up = *metadata;
- }
- }
-
- DelayedAddObjCClassProperty(const DelayedAddObjCClassProperty &rhs) {
- *this = rhs;
- }
-
- DelayedAddObjCClassProperty &
- operator=(const DelayedAddObjCClassProperty &rhs) {
- m_class_opaque_type = rhs.m_class_opaque_type;
- m_property_name = rhs.m_property_name;
- m_property_opaque_type = rhs.m_property_opaque_type;
- m_property_setter_name = rhs.m_property_setter_name;
- m_property_getter_name = rhs.m_property_getter_name;
- m_property_attributes = rhs.m_property_attributes;
-
- if (rhs.m_metadata_up) {
- m_metadata_up = std::make_unique<ClangASTMetadata>();
- *m_metadata_up = *rhs.m_metadata_up;
- }
- return *this;
- }
+ m_property_attributes(property_attributes), m_metadata(metadata) {}
bool Finalize() {
return TypeSystemClang::AddObjCClassProperty(
m_class_opaque_type, m_property_name, m_property_opaque_type,
/*ivar_decl=*/nullptr, m_property_setter_name, m_property_getter_name,
- m_property_attributes, m_metadata_up.get());
+ m_property_attributes, m_metadata);
}
private:
@@ -1929,7 +1904,7 @@ class DWARFASTParserClang::DelayedAddObjCClassProperty {
const char *m_property_setter_name;
const char *m_property_getter_name;
uint32_t m_property_attributes;
- std::unique_ptr<ClangASTMetadata> m_metadata_up;
+ ClangASTMetadata m_metadata;
};
bool DWARFASTParserClang::ParseTemplateDIE(
@@ -2721,10 +2696,10 @@ void DWARFASTParserClang::ParseObjCProperty(
ClangASTMetadata metadata;
metadata.SetUserID(die.GetID());
- delayed_properties.push_back(DelayedAddObjCClassProperty(
+ delayed_properties.emplace_back(
class_clang_type, propAttrs.prop_name,
member_type->GetLayoutCompilerType(), propAttrs.prop_setter_name,
- propAttrs.prop_getter_name, propAttrs.prop_attributes, &metadata));
+ propAttrs.prop_getter_name, propAttrs.prop_attributes, metadata);
}
llvm::Expected<llvm::APInt> DWARFASTParserClang::ExtractIntFromFormValue(
diff --git a/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp b/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
index b79d3e63f72b1..0c71df625ae34 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
@@ -618,7 +618,7 @@ clang::QualType PdbAstBuilder::CreateRecordType(PdbTypeSymId id,
CompilerType ct = m_clang.CreateRecordType(
context, OptionalClangModuleID(), access, uname, llvm::to_underlying(ttk),
- lldb::eLanguageTypeC_plus_plus, &metadata);
+ lldb::eLanguageTypeC_plus_plus, metadata);
lldbassert(ct.IsValid());
diff --git a/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp b/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
index 17c5f6118603f..807ee5b30779c 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
@@ -366,7 +366,7 @@ UdtRecordCompleter::AddMember(TypeSystemClang &clang, Member *field,
metadata.SetIsDynamicCXXType(false);
CompilerType record_ct = clang.CreateRecordType(
parent_decl_ctx, OptionalClangModuleID(), lldb::eAccessPublic, "",
- llvm::to_underlying(kind), lldb::eLanguageTypeC_plus_plus, &metadata);
+ llvm::to_underlying(kind), lldb::eLanguageTypeC_plus_plus, metadata);
TypeSystemClang::StartTagDeclarationDefinition(record_ct);
ClangASTImporter::LayoutInfo layout;
clang::DeclContext *decl_ctx = clang.GetDeclContextForType(record_ct);
diff --git a/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp b/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
index d656ca3facf73..fa3530a0c22ff 100644
--- a/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
+++ b/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
@@ -420,7 +420,7 @@ lldb::TypeSP PDBASTParser::CreateLLDBTypeFromPDBType(const PDBSymbol &type) {
clang_type = m_ast.CreateRecordType(
decl_context, OptionalClangModuleID(), access, name, tag_type_kind,
- lldb::eLanguageTypeC_plus_plus, &metadata);
+ lldb::eLanguageTypeC_plus_plus, metadata);
assert(clang_type.IsValid());
auto record_decl =
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 218402afd12a5..66394665d29c5 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -1220,7 +1220,8 @@ TypeSystemClang::GetOrCreateClangModule(llvm::StringRef name,
CompilerType TypeSystemClang::CreateRecordType(
clang::DeclContext *decl_ctx, OptionalClangModuleID owning_module,
AccessType access_type, llvm::StringRef name, int kind,
- LanguageType language, ClangASTMetadata *metadata, bool exports_symbols) {
+ LanguageType language, std::optional<ClangASTMetadata> metadata,
+ bool exports_symbols) {
ASTContext &ast = getASTContext();
if (decl_ctx == nullptr)
@@ -1799,7 +1800,7 @@ bool TypeSystemClang::RecordHasFields(const RecordDecl *record_decl) {
CompilerType TypeSystemClang::CreateObjCClass(
llvm::StringRef name, clang::DeclContext *decl_ctx,
OptionalClangModuleID owning_module, bool isForwardDecl, bool isInternal,
- ClangASTMetadata *metadata) {
+ std::optional<ClangASTMetadata> metadata) {
ASTContext &ast = getASTContext();
assert(!name.empty());
if (!decl_ctx)
@@ -7986,7 +7987,7 @@ bool TypeSystemClang::AddObjCClassProperty(
const CompilerType &type, const char *property_name,
const CompilerType &property_clang_type, clang::ObjCIvarDecl *ivar_decl,
const char *property_setter_name, const char *property_getter_name,
- uint32_t property_attributes, ClangASTMetadata *metadata) {
+ uint32_t property_attributes, ClangASTMetadata metadata) {
if (!type || !property_clang_type.IsValid() || property_name == nullptr ||
property_name[0] == '\0')
return false;
@@ -8030,8 +8031,7 @@ bool TypeSystemClang::AddObjCClassProperty(
if (!property_decl)
return false;
- if (metadata)
- ast->SetMetadata(property_decl, *metadata);
+ ast->SetMetadata(property_decl, metadata);
class_interface_decl->addDecl(property_decl);
@@ -8123,8 +8123,7 @@ bool TypeSystemClang::AddObjCClassProperty(
SetMemberOwningModule(getter, class_interface_decl);
if (getter) {
- if (metadata)
- ast->SetMetadata(getter, *metadata);
+ ast->SetMetadata(getter, metadata);
getter->setMethodParams(clang_ast, llvm::ArrayRef<clang::ParmVarDecl *>(),
llvm::ArrayRef<clang::SourceLocation>());
@@ -8166,8 +8165,7 @@ bool TypeSystemClang::AddObjCClassProperty(
SetMemberOwningModule(setter, class_interface_decl);
if (setter) {
- if (metadata)
- ast->SetMetadata(setter, *metadata);
+ ast->SetMetadata(setter, metadata);
llvm::SmallVector<clang::ParmVarDecl *, 1> params;
params.push_back(clang::ParmVarDecl::Create(
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
index 0aec1bcaaf9a7..789352750b222 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -29,6 +29,7 @@
#include "llvm/ADT/APSInt.h"
#include "llvm/ADT/SmallVector.h"
+#include "Plugins/ExpressionParser/Clang/ClangASTMetadata.h"
#include "Plugins/ExpressionParser/Clang/ClangPersistentVariables.h"
#include "lldb/Expression/ExpressionVariable.h"
#include "lldb/Symbol/CompilerType.h"
@@ -50,7 +51,6 @@ class ModuleMap;
namespace lldb_private {
-class ClangASTMetadata;
class ClangASTSource;
class Declaration;
@@ -325,13 +325,13 @@ class TypeSystemClang : public TypeSystem {
bool is_framework = false,
bool is_explicit = false);
- CompilerType CreateRecordType(clang::DeclContext *decl_ctx,
- OptionalClangModuleID owning_module,
- lldb::AccessType access_type,
- llvm::StringRef name, int kind,
- lldb::LanguageType language,
- ClangASTMetadata *metadata = nullptr,
- bool exports_symbols = false);
+ CompilerType
+ CreateRecordType(clang::DeclContext *decl_ctx,
+ OptionalClangModuleID owning_module,
+ lldb::AccessType access_type, llvm::StringRef name, int kind,
+ lldb::LanguageType language,
+ std::optional<ClangASTMetadata> metadata = std::nullopt,
+ bool exports_symbols = false);
class TemplateParameterInfos {
public:
@@ -455,11 +455,11 @@ class TypeSystemClang : public TypeSystem {
bool BaseSpecifierIsEmpty(const clang::CXXBaseSpecifier *b);
- CompilerType CreateObjCClass(llvm::StringRef name,
- clang::DeclContext *decl_ctx,
- OptionalClangModuleID owning_module,
- bool isForwardDecl, bool isInternal,
- ClangASTMetadata *metadata = nullptr);
+ CompilerType
+ CreateObjCClass(llvm::StringRef name, clang::DeclContext *decl_ctx,
+ OptionalClangModuleID owning_module, bool isForwardDecl,
+ bool isInternal,
+ std::optional<ClangASTMetadata> metadata = std::nullopt);
// Returns a mask containing bits from the TypeSystemClang::eTypeXXX
// enumerations
@@ -1005,7 +1005,7 @@ class TypeSystemClang : public TypeSystem {
const char *property_setter_name,
const char *property_getter_name,
uint32_t property_attributes,
- ClangASTMetadata *metadata);
+ ClangASTMetadata metadata);
static clang::ObjCMethodDecl *AddMethodToObjCObjectType(
const CompilerType &type,
>From fada2b82ce478866f133a10f84e04dfe0ed85c37 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Wed, 7 Aug 2024 12:49:39 +0100
Subject: [PATCH 2/2] fixup! fix unit-test build-failures
---
lldb/unittests/Symbol/TestTypeSystemClang.cpp | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/lldb/unittests/Symbol/TestTypeSystemClang.cpp b/lldb/unittests/Symbol/TestTypeSystemClang.cpp
index 30d20b9587f91..2c2dae01e1b07 100644
--- a/lldb/unittests/Symbol/TestTypeSystemClang.cpp
+++ b/lldb/unittests/Symbol/TestTypeSystemClang.cpp
@@ -296,7 +296,7 @@ TEST_F(TestTypeSystemClang, TestOwningModule) {
CompilerType record_type = ast.CreateRecordType(
nullptr, OptionalClangModuleID(200), lldb::eAccessPublic, "FooRecord",
llvm::to_underlying(clang::TagTypeKind::Struct),
- lldb::eLanguageTypeC_plus_plus, nullptr);
+ lldb::eLanguageTypeC_plus_plus, std::nullopt);
auto *rd = TypeSystemClang::GetAsRecordDecl(record_type);
EXPECT_FALSE(!rd);
EXPECT_EQ(rd->getOwningModuleID(), 200u);
@@ -317,7 +317,7 @@ TEST_F(TestTypeSystemClang, TestIsClangType) {
CompilerType record_type = m_ast->CreateRecordType(
nullptr, OptionalClangModuleID(100), lldb::eAccessPublic, "FooRecord",
llvm::to_underlying(clang::TagTypeKind::Struct),
- lldb::eLanguageTypeC_plus_plus, nullptr);
+ lldb::eLanguageTypeC_plus_plus, std::nullopt);
// Clang builtin type and record type should pass
EXPECT_TRUE(ClangUtil::IsClangType(bool_type));
EXPECT_TRUE(ClangUtil::IsClangType(record_type));
@@ -330,7 +330,7 @@ TEST_F(TestTypeSystemClang, TestRemoveFastQualifiers) {
CompilerType record_type = m_ast->CreateRecordType(
nullptr, OptionalClangModuleID(), lldb::eAccessPublic, "FooRecord",
llvm::to_underlying(clang::TagTypeKind::Struct),
- lldb::eLanguageTypeC_plus_plus, nullptr);
+ lldb::eLanguageTypeC_plus_plus, std::nullopt);
QualType qt;
qt = ClangUtil::GetQualType(record_type);
@@ -403,7 +403,7 @@ TEST_F(TestTypeSystemClang, TestRecordHasFields) {
CompilerType empty_base = m_ast->CreateRecordType(
nullptr, OptionalClangModuleID(), lldb::eAccessPublic, "EmptyBase",
llvm::to_underlying(clang::TagTypeKind::Struct),
- lldb::eLanguageTypeC_plus_plus, nullptr);
+ lldb::eLanguageTypeC_plus_plus, std::nullopt);
TypeSystemClang::StartTagDeclarationDefinition(empty_base);
TypeSystemClang::CompleteTagDeclarationDefinition(empty_base);
@@ -415,7 +415,7 @@ TEST_F(TestTypeSystemClang, TestRecordHasFields) {
CompilerType non_empty_base = m_ast->CreateRecordType(
nullptr, OptionalClangModuleID(), lldb::eAccessPublic, "NonEmptyBase",
llvm::to_underlying(clang::TagTypeKind::Struct),
- lldb::eLanguageTypeC_plus_plus, nullptr);
+ lldb::eLanguageTypeC_plus_plus, std::nullopt);
TypeSystemClang::StartTagDeclarationDefinition(non_empty_base);
FieldDecl *non_empty_base_field_decl = m_ast->AddFieldToRecordType(
non_empty_base, "MyField", int_type, eAccessPublic, 0);
@@ -432,7 +432,7 @@ TEST_F(TestTypeSystemClang, TestRecordHasFields) {
CompilerType empty_derived = m_ast->CreateRecordType(
nullptr, OptionalClangModuleID(), lldb::eAccessPublic, "EmptyDerived",
llvm::to_underlying(clang::TagTypeKind::Struct),
- lldb::eLanguageTypeC_plus_plus, nullptr);
+ lldb::eLanguageTypeC_plus_plus, std::nullopt);
TypeSystemClang::StartTagDeclarationDefinition(empty_derived);
std::unique_ptr<clang::CXXBaseSpecifier> non_empty_base_spec =
m_ast->CreateBaseClassSpecifier(non_empty_base.GetOpaqueQualType(),
@@ -455,7 +455,7 @@ TEST_F(TestTypeSystemClang, TestRecordHasFields) {
CompilerType empty_derived2 = m_ast->CreateRecordType(
nullptr, OptionalClangModuleID(), lldb::eAccessPublic, "EmptyDerived2",
llvm::to_underlying(clang::TagTypeKind::Struct),
- lldb::eLanguageTypeC_plus_plus, nullptr);
+ lldb::eLanguageTypeC_plus_plus, std::nullopt);
TypeSystemClang::StartTagDeclarationDefinition(empty_derived2);
std::unique_ptr<CXXBaseSpecifier> non_empty_vbase_spec =
m_ast->CreateBaseClassSpecifier(non_empty_base.GetOpaqueQualType(),
More information about the lldb-commits
mailing list