[Lldb-commits] [PATCH] D156498: [lldb] Support recursive record types in CTF

Michael Buch via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Fri Jul 28 07:18:55 PDT 2023

Michael137 added a comment.

Generally LGTM, just some clarifications questions

Comment at: lldb/source/Plugins/SymbolFile/CTF/SymbolFileCTF.cpp:508
                               ctf_record.name.data(), tag_kind, eLanguageTypeC);
+  m_compiler_types[record_type.GetOpaqueQualType()] = &ctf_record;
+  Declaration decl;
Just to clarify the lifetimes. This `ctf_record` lives on `m_ast` while the `m_compiler_types` on the SymbolFile, so we're guaranteed that the `ctf_record` lives long enough?

Comment at: lldb/source/Plugins/SymbolFile/CTF/SymbolFileCTF.cpp:524-529
+  // We only support resolving record types.
+  assert(ctf_type->kind == CTFType::Kind::eStruct ||
+         ctf_type->kind == CTFType::Kind::eUnion);
-  m_ast->StartTagDeclarationDefinition(record_type);
-  for (const CTFRecord::Field &field : ctf_record.fields) {
-    if (Type *field_type = ResolveTypeUID(field.type)) {
-      const uint32_t field_size = field_type->GetByteSize(nullptr).value_or(0);
-      TypeSystemClang::AddFieldToRecordType(record_type, field.name,
-                                            field_type->GetFullCompilerType(),
-                                            eAccessPublic, field_size);
+  // Cast to the appropriate CTF type.
+  const CTFRecord *ctf_record = static_cast<const CTFRecord *>(ctf_type);
Nit: would it make sense to just add `classof` methods to `CTFType` and use the llvm cast facilities?

Feel free to ignore since there's just one instance of such cast afaict



More information about the lldb-commits mailing list