[Lldb-commits] [lldb] [lldb][TypeSystem] Pass ClangASTMetadata by-value when creating record/objc types (PR #102296)
via lldb-commits
lldb-commits at lists.llvm.org
Wed Aug 7 03:11:22 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: Michael Buch (Michael137)
<details>
<summary>Changes</summary>
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. This meant we could also get rid off the user-provided copy constructors.
---
Full diff: https://github.com/llvm/llvm-project/pull/102296.diff
6 Files Affected:
- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp (+7-32)
- (modified) lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp (+1-1)
- (modified) lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp (+1-1)
- (modified) lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp (+1-1)
- (modified) lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp (+7-9)
- (modified) lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h (+14-14)
``````````diff
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index a4dcde1629fc21..1a13725b99804a 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 b79d3e63f72b1d..0c71df625ae348 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 17c5f6118603f4..807ee5b30779c9 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 d656ca3facf73d..fa3530a0c22ff3 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 218402afd12a51..66394665d29c5f 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 0aec1bcaaf9a75..789352750b222e 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,
``````````
</details>
https://github.com/llvm/llvm-project/pull/102296
More information about the lldb-commits
mailing list