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 <michaelbuc...@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 <michaelbuc...@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(), _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
