[lldb][TypeSystem] Pass ClangASTMetadata by-value when creating record/objc types #102296
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…d/objc types This is a follow-up to llvm#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.
|
@llvm/pr-subscribers-lldb Author: Michael Buch (Michael137) ChangesThis is a follow-up to #102161 where we changed the This patch continues from there and changes As a drive-by change, I also changed Full diff: https://github.com/llvm/llvm-project/pull/102296.diff 6 Files Affected:
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,
|
labath
approved these changes
Aug 7, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a follow-up to #102161 where we changed the
GetMetadata/SetMetadataAPIs to passClangASTMetadataby-value, instead ofClangASTMetadata *, which wasn't a very friendly API.This patch continues from there and changes
CreateRecordType/CreateObjCClassto take the metadata by-value as well.As a drive-by change, I also changed
DelayedAddObjCClassPropertyto store the metadata by-value, instead of in astd::unique_ptr, which AFAICT, was done solely due to the TypeSystemClang APIs taking the metadata by pointer. This meant we could also get rid of the user-provided copy constructors.