[Lldb-commits] [lldb] f9f0ae1 - [lldb][TypeSystemClang] Pass ClangASTMetadata around by value (#102161)
via lldb-commits
lldb-commits at lists.llvm.org
Tue Aug 6 13:42:04 PDT 2024
Author: Michael Buch
Date: 2024-08-06T21:41:59+01:00
New Revision: f9f0ae1bc47fbe76141cce63a6e92e3f3546ec9b
URL: https://github.com/llvm/llvm-project/commit/f9f0ae1bc47fbe76141cce63a6e92e3f3546ec9b
DIFF: https://github.com/llvm/llvm-project/commit/f9f0ae1bc47fbe76141cce63a6e92e3f3546ec9b.diff
LOG: [lldb][TypeSystemClang] Pass ClangASTMetadata around by value (#102161)
This patch changes the return type of `GetMetadata` from a
`ClangASTMetadata*` to a `std::optional<ClangASTMetadata>`. Except for
one call-site (`SetDeclIsForcefullyCompleted`), we never actually make
use of the mutability of the returned metadata. And we never make use of
the pointer-identity. By passing `ClangASTMetadata` by-value (the type
is fairly small, size of 2 64-bit pointers) we'll avoid some questions
surrounding the lifetimes/ownership/mutability of this metadata.
For consistency, we also change the parameter to `SetMetadata` from
`ClangASTMetadata&` to `ClangASTMetadata` (which is an NFC since we copy
the data anyway).
This came up during some changes we plan to make where we [create
redeclaration chains for decls in the LLDB
AST](https://github.com/llvm/llvm-project/pull/95100). We want to avoid
having to dig out the canonical decl of the declaration chain for
retrieving/setting the metadata. It should just be copied across all
decls in the chain. This is easier to guarantee when everything is done
by-value.
Added:
Modified:
lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.h
lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
lldb/unittests/Symbol/TestClangASTImporter.cpp
Removed:
################################################################################
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
index 44071d1ea71c7..adf13ff736adc 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
@@ -84,8 +84,7 @@ clang::Decl *ClangASTImporter::CopyDecl(clang::ASTContext *dst_ast,
LLDB_LOG_ERROR(log, result.takeError(), "Couldn't import decl: {0}");
if (log) {
lldb::user_id_t user_id = LLDB_INVALID_UID;
- ClangASTMetadata *metadata = GetDeclMetadata(decl);
- if (metadata)
+ if (std::optional<ClangASTMetadata> metadata = GetDeclMetadata(decl))
user_id = metadata->GetUserID();
if (NamedDecl *named_decl = dyn_cast<NamedDecl>(decl))
@@ -950,7 +949,8 @@ bool ClangASTImporter::RequireCompleteType(clang::QualType type) {
return true;
}
-ClangASTMetadata *ClangASTImporter::GetDeclMetadata(const clang::Decl *decl) {
+std::optional<ClangASTMetadata>
+ClangASTImporter::GetDeclMetadata(const clang::Decl *decl) {
DeclOrigin decl_origin = GetDeclOrigin(decl);
if (decl_origin.Valid()) {
@@ -1105,7 +1105,7 @@ ClangASTImporter::ASTImporterDelegate::ImportImpl(Decl *From) {
// If we have a forcefully completed type, try to find an actual definition
// for it in other modules.
- const ClangASTMetadata *md = m_main.GetDeclMetadata(From);
+ std::optional<ClangASTMetadata> md = m_main.GetDeclMetadata(From);
auto *td = dyn_cast<TagDecl>(From);
if (td && md && md->IsForcefullyCompleted()) {
Log *log = GetLog(LLDBLog::Expressions);
@@ -1284,8 +1284,7 @@ void ClangASTImporter::ASTImporterDelegate::Imported(clang::Decl *from,
}
lldb::user_id_t user_id = LLDB_INVALID_UID;
- ClangASTMetadata *metadata = m_main.GetDeclMetadata(from);
- if (metadata)
+ if (std::optional<ClangASTMetadata> metadata = m_main.GetDeclMetadata(from))
user_id = metadata->GetUserID();
if (log) {
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.h b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.h
index bc962e544d2f1..6231f0fb25939 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.h
@@ -189,7 +189,7 @@ class ClangASTImporter {
/// is only a shallow clone that lacks any contents.
void SetDeclOrigin(const clang::Decl *decl, clang::Decl *original_decl);
- ClangASTMetadata *GetDeclMetadata(const clang::Decl *decl);
+ std::optional<ClangASTMetadata> GetDeclMetadata(const clang::Decl *decl);
//
// Namespace maps
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
index 35038a56440d5..f096566c0cf20 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
@@ -219,9 +219,10 @@ void ClangUserExpression::ScanContext(ExecutionContext &exe_ctx, Status &err) {
// whatever runtime the debug info says the object pointer belongs to. Do
// that here.
- ClangASTMetadata *metadata =
- TypeSystemClang::DeclContextGetMetaData(decl_context, function_decl);
- if (metadata && metadata->HasObjectPtr()) {
+ if (std::optional<ClangASTMetadata> metadata =
+ TypeSystemClang::DeclContextGetMetaData(decl_context,
+ function_decl);
+ metadata && metadata->HasObjectPtr()) {
lldb::LanguageType language = metadata->GetObjectPtrLanguage();
if (language == lldb::eLanguageTypeC_plus_plus) {
if (m_enforce_valid_object) {
diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
index 6894cdccaf95a..f3a008ff1e893 100644
--- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
@@ -398,9 +398,9 @@ bool AppleObjCDeclVendor::FinishDecl(clang::ObjCInterfaceDecl *interface_decl) {
Log *log(
GetLog(LLDBLog::Expressions)); // FIXME - a more appropriate log channel?
- ClangASTMetadata *metadata = m_ast_ctx->GetMetadata(interface_decl);
ObjCLanguageRuntime::ObjCISA objc_isa = 0;
- if (metadata)
+ if (std::optional<ClangASTMetadata> metadata =
+ m_ast_ctx->GetMetadata(interface_decl))
objc_isa = metadata->GetISAPtr();
if (!objc_isa)
@@ -559,8 +559,8 @@ uint32_t AppleObjCDeclVendor::FindDecls(ConstString name, bool append,
ast_ctx.getObjCInterfaceType(result_iface_decl);
uint64_t isa_value = LLDB_INVALID_ADDRESS;
- ClangASTMetadata *metadata = m_ast_ctx->GetMetadata(result_iface_decl);
- if (metadata)
+ if (std::optional<ClangASTMetadata> metadata =
+ m_ast_ctx->GetMetadata(result_iface_decl))
isa_value = metadata->GetISAPtr();
LLDB_LOGF(log,
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 78e9eeb7bc7aa..218402afd12a5 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -1787,8 +1787,8 @@ bool TypeSystemClang::RecordHasFields(const RecordDecl *record_decl) {
// -flimit-debug-info instead of just seeing nothing if this is a base class
// (since we were hiding empty base classes), or nothing when you turn open
// an valiable whose type was incomplete.
- ClangASTMetadata *meta_data = GetMetadata(record_decl);
- if (meta_data && meta_data->IsForcefullyCompleted())
+ if (std::optional<ClangASTMetadata> meta_data = GetMetadata(record_decl);
+ meta_data && meta_data->IsForcefullyCompleted())
return true;
return false;
@@ -2465,27 +2465,31 @@ void TypeSystemClang::SetMetadataAsUserID(const clang::Type *type,
}
void TypeSystemClang::SetMetadata(const clang::Decl *object,
- ClangASTMetadata &metadata) {
+ ClangASTMetadata metadata) {
m_decl_metadata[object] = metadata;
}
void TypeSystemClang::SetMetadata(const clang::Type *object,
- ClangASTMetadata &metadata) {
+ ClangASTMetadata metadata) {
m_type_metadata[object] = metadata;
}
-ClangASTMetadata *TypeSystemClang::GetMetadata(const clang::Decl *object) {
+std::optional<ClangASTMetadata>
+TypeSystemClang::GetMetadata(const clang::Decl *object) {
auto It = m_decl_metadata.find(object);
if (It != m_decl_metadata.end())
- return &It->second;
- return nullptr;
+ return It->second;
+
+ return std::nullopt;
}
-ClangASTMetadata *TypeSystemClang::GetMetadata(const clang::Type *object) {
+std::optional<ClangASTMetadata>
+TypeSystemClang::GetMetadata(const clang::Type *object) {
auto It = m_type_metadata.find(object);
if (It != m_type_metadata.end())
- return &It->second;
- return nullptr;
+ return It->second;
+
+ return std::nullopt;
}
void TypeSystemClang::SetCXXRecordDeclAccess(const clang::CXXRecordDecl *object,
@@ -2934,9 +2938,10 @@ bool TypeSystemClang::IsRuntimeGeneratedType(
clang::ObjCInterfaceDecl *result_iface_decl =
llvm::dyn_cast<clang::ObjCInterfaceDecl>(decl_ctx);
- ClangASTMetadata *ast_metadata = GetMetadata(result_iface_decl);
+ std::optional<ClangASTMetadata> ast_metadata = GetMetadata(result_iface_decl);
if (!ast_metadata)
return false;
+
return (ast_metadata->GetISAPtr() != 0);
}
@@ -3622,8 +3627,8 @@ bool TypeSystemClang::IsPossibleDynamicType(lldb::opaque_compiler_type_t type,
if (is_complete)
success = cxx_record_decl->isDynamicClass();
else {
- ClangASTMetadata *metadata = GetMetadata(cxx_record_decl);
- if (metadata)
+ if (std::optional<ClangASTMetadata> metadata =
+ GetMetadata(cxx_record_decl))
success = metadata->GetIsDynamicCXXType();
else {
is_complete = GetType(pointee_qual_type).GetCompleteType();
@@ -5326,7 +5331,8 @@ GetDynamicArrayInfo(TypeSystemClang &ast, SymbolFile *sym_file,
clang::QualType qual_type,
const ExecutionContext *exe_ctx) {
if (qual_type->isIncompleteArrayType())
- if (auto *metadata = ast.GetMetadata(qual_type.getTypePtr()))
+ if (std::optional<ClangASTMetadata> metadata =
+ ast.GetMetadata(qual_type.getTypePtr()))
return sym_file->GetDynamicArrayInfoForUID(metadata->GetUserID(),
exe_ctx);
return std::nullopt;
@@ -8868,8 +8874,7 @@ void TypeSystemClang::DumpTypeDescription(lldb::opaque_compiler_type_t type,
CompilerType ct(weak_from_this(), type);
const clang::Type *clang_type = ClangUtil::GetQualType(ct).getTypePtr();
- ClangASTMetadata *metadata = GetMetadata(clang_type);
- if (metadata) {
+ if (std::optional<ClangASTMetadata> metadata = GetMetadata(clang_type)) {
metadata->Dump(&s);
}
}
@@ -9490,7 +9495,7 @@ bool TypeSystemClang::DeclContextIsClassMethod(void *opaque_decl_ctx) {
return true;
} else if (clang::FunctionDecl *fun_decl =
llvm::dyn_cast<clang::FunctionDecl>(decl_ctx)) {
- if (ClangASTMetadata *metadata = GetMetadata(fun_decl))
+ if (std::optional<ClangASTMetadata> metadata = GetMetadata(fun_decl))
return metadata->HasObjectPtr();
}
@@ -9543,7 +9548,7 @@ TypeSystemClang::DeclContextGetLanguage(void *opaque_decl_ctx) {
} else if (llvm::isa<clang::CXXMethodDecl>(decl_ctx)) {
return eLanguageTypeC_plus_plus;
} else if (auto *fun_decl = llvm::dyn_cast<clang::FunctionDecl>(decl_ctx)) {
- if (ClangASTMetadata *metadata = GetMetadata(fun_decl))
+ if (std::optional<ClangASTMetadata> metadata = GetMetadata(fun_decl))
return metadata->GetObjectPtrLanguage();
}
@@ -9593,7 +9598,7 @@ TypeSystemClang::DeclContextGetAsNamespaceDecl(const CompilerDeclContext &dc) {
return nullptr;
}
-ClangASTMetadata *
+std::optional<ClangASTMetadata>
TypeSystemClang::DeclContextGetMetaData(const CompilerDeclContext &dc,
const Decl *object) {
TypeSystemClang *ast = llvm::cast<TypeSystemClang>(dc.GetTypeSystem());
@@ -9827,8 +9832,7 @@ bool TypeSystemClang::IsForcefullyCompleted(lldb::opaque_compiler_type_t type) {
if (record_type) {
const clang::RecordDecl *record_decl = record_type->getDecl();
assert(record_decl);
- ClangASTMetadata *metadata = GetMetadata(record_decl);
- if (metadata)
+ if (std::optional<ClangASTMetadata> metadata = GetMetadata(record_decl))
return metadata->IsForcefullyCompleted();
}
}
@@ -9838,11 +9842,13 @@ bool TypeSystemClang::IsForcefullyCompleted(lldb::opaque_compiler_type_t type) {
bool TypeSystemClang::SetDeclIsForcefullyCompleted(const clang::TagDecl *td) {
if (td == nullptr)
return false;
- ClangASTMetadata *metadata = GetMetadata(td);
- if (metadata == nullptr)
+ std::optional<ClangASTMetadata> metadata = GetMetadata(td);
+ if (!metadata)
return false;
m_has_forcefully_completed_types = true;
metadata->SetIsForcefullyCompleted();
+ SetMetadata(td, *metadata);
+
return true;
}
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
index 56a5c0a516706..0aec1bcaaf9a7 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -191,11 +191,11 @@ class TypeSystemClang : public TypeSystem {
void SetMetadataAsUserID(const clang::Decl *decl, lldb::user_id_t user_id);
void SetMetadataAsUserID(const clang::Type *type, lldb::user_id_t user_id);
- void SetMetadata(const clang::Decl *object, ClangASTMetadata &meta_data);
+ void SetMetadata(const clang::Decl *object, ClangASTMetadata meta_data);
- void SetMetadata(const clang::Type *object, ClangASTMetadata &meta_data);
- ClangASTMetadata *GetMetadata(const clang::Decl *object);
- ClangASTMetadata *GetMetadata(const clang::Type *object);
+ void SetMetadata(const clang::Type *object, ClangASTMetadata meta_data);
+ std::optional<ClangASTMetadata> GetMetadata(const clang::Decl *object);
+ std::optional<ClangASTMetadata> GetMetadata(const clang::Type *object);
void SetCXXRecordDeclAccess(const clang::CXXRecordDecl *object,
clang::AccessSpecifier access);
@@ -616,8 +616,9 @@ class TypeSystemClang : public TypeSystem {
static clang::NamespaceDecl *
DeclContextGetAsNamespaceDecl(const CompilerDeclContext &dc);
- static ClangASTMetadata *DeclContextGetMetaData(const CompilerDeclContext &dc,
- const clang::Decl *object);
+ static std::optional<ClangASTMetadata>
+ DeclContextGetMetaData(const CompilerDeclContext &dc,
+ const clang::Decl *object);
static clang::ASTContext *
DeclContextGetTypeSystemClang(const CompilerDeclContext &dc);
diff --git a/lldb/unittests/Symbol/TestClangASTImporter.cpp b/lldb/unittests/Symbol/TestClangASTImporter.cpp
index de59efe533eb9..41c7ed75155f3 100644
--- a/lldb/unittests/Symbol/TestClangASTImporter.cpp
+++ b/lldb/unittests/Symbol/TestClangASTImporter.cpp
@@ -188,7 +188,7 @@ TEST_F(TestClangASTImporter, MetadataPropagation) {
ASSERT_NE(nullptr, imported);
// Check that we got the same Metadata.
- ASSERT_NE(nullptr, importer.GetDeclMetadata(imported));
+ ASSERT_NE(std::nullopt, importer.GetDeclMetadata(imported));
EXPECT_EQ(metadata, importer.GetDeclMetadata(imported)->GetUserID());
}
@@ -219,7 +219,7 @@ TEST_F(TestClangASTImporter, MetadataPropagationIndirectImport) {
ASSERT_NE(nullptr, imported);
// Check that we got the same Metadata.
- ASSERT_NE(nullptr, importer.GetDeclMetadata(imported));
+ ASSERT_NE(std::nullopt, importer.GetDeclMetadata(imported));
EXPECT_EQ(metadata, importer.GetDeclMetadata(imported)->GetUserID());
}
@@ -244,7 +244,7 @@ TEST_F(TestClangASTImporter, MetadataPropagationAfterCopying) {
source.ast->SetMetadataAsUserID(source.record_decl, metadata);
// Check that we got the same Metadata.
- ASSERT_NE(nullptr, importer.GetDeclMetadata(imported));
+ ASSERT_NE(std::nullopt, importer.GetDeclMetadata(imported));
EXPECT_EQ(metadata, importer.GetDeclMetadata(imported)->GetUserID());
}
More information about the lldb-commits
mailing list