Index: include/lldb/Symbol/ClangASTContext.h =================================================================== --- include/lldb/Symbol/ClangASTContext.h +++ include/lldb/Symbol/ClangASTContext.h @@ -858,18 +858,14 @@ void AddMethodOverridesForCXXRecordType(lldb::opaque_compiler_type_t type); // C++ Base Classes - clang::CXXBaseSpecifier * + std::unique_ptr CreateBaseClassSpecifier(lldb::opaque_compiler_type_t type, lldb::AccessType access, bool is_virtual, bool base_of_class); - static void DeleteBaseClassSpecifiers(clang::CXXBaseSpecifier **base_classes, - unsigned num_base_classes); - - bool - SetBaseClassesForClassType(lldb::opaque_compiler_type_t type, - clang::CXXBaseSpecifier const *const *base_classes, - unsigned num_base_classes); + bool TransferBaseClasses( + lldb::opaque_compiler_type_t type, + std::vector> bases); static bool SetObjCSuperClass(const CompilerType &type, const CompilerType &superclass_compiler_type); Index: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h =================================================================== --- source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h +++ source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h @@ -80,16 +80,16 @@ lldb_private::ClangASTContext::TemplateParameterInfos &template_param_infos); - bool - ParseChildMembers(const lldb_private::SymbolContext &sc, const DWARFDIE &die, - lldb_private::CompilerType &class_compiler_type, - const lldb::LanguageType class_language, - std::vector &base_classes, - std::vector &member_accessibilities, - DWARFDIECollection &member_function_dies, - DelayedPropertyList &delayed_properties, - lldb::AccessType &default_accessibility, bool &is_a_class, - lldb_private::ClangASTImporter::LayoutInfo &layout_info); + bool ParseChildMembers( + const lldb_private::SymbolContext &sc, const DWARFDIE &die, + lldb_private::CompilerType &class_compiler_type, + const lldb::LanguageType class_language, + std::vector> &base_classes, + std::vector &member_accessibilities, + DWARFDIECollection &member_function_dies, + DelayedPropertyList &delayed_properties, + lldb::AccessType &default_accessibility, bool &is_a_class, + lldb_private::ClangASTImporter::LayoutInfo &layout_info); size_t ParseChildParameters(const lldb_private::SymbolContext &sc, Index: source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp =================================================================== --- source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -2184,14 +2184,14 @@ } SymbolContext sc(die.GetLLDBCompileUnit()); - std::vector base_classes; + std::vector> bases; std::vector member_accessibilities; bool is_a_class = false; // Parse members and base classes first DWARFDIECollection member_function_dies; DelayedPropertyList delayed_properties; - ParseChildMembers(sc, die, clang_type, class_language, base_classes, + ParseChildMembers(sc, die, clang_type, class_language, bases, member_accessibilities, member_function_dies, delayed_properties, default_accessibility, is_a_class, layout_info); @@ -2255,11 +2255,11 @@ &member_accessibilities.front(), member_accessibilities.size()); } - if (!base_classes.empty()) { + if (!bases.empty()) { // Make sure all base classes refer to complete types and not forward // declarations. If we don't do this, clang will crash with an - // assertion in the call to clang_type.SetBaseClassesForClassType() - for (auto &base_class : base_classes) { + // assertion in the call to clang_type.TransferBaseClasses() + for (const auto &base_class : bases) { clang::TypeSourceInfo *type_source_info = base_class->getTypeSourceInfo(); if (type_source_info) { @@ -2278,7 +2278,7 @@ // We have no choice other than to pretend that the base class // is complete. If we don't do this, clang will crash when we // call setBases() inside of - // "clang_type.SetBaseClassesForClassType()" below. Since we + // "clang_type.TransferBaseClasses()" below. Since we // provide layout assistance, all ivars in this class and other // classes will be fine, this is the best we can do short of // crashing. @@ -2290,14 +2290,9 @@ } } } - m_ast.SetBaseClassesForClassType(clang_type.GetOpaqueQualType(), - &base_classes.front(), - base_classes.size()); - - // Clang will copy each CXXBaseSpecifier in "base_classes" so we have - // to free them all. - ClangASTContext::DeleteBaseClassSpecifiers(&base_classes.front(), - base_classes.size()); + + m_ast.TransferBaseClasses(clang_type.GetOpaqueQualType(), + std::move(bases)); } } } @@ -2675,7 +2670,7 @@ bool DWARFASTParserClang::ParseChildMembers( const SymbolContext &sc, const DWARFDIE &parent_die, CompilerType &class_clang_type, const LanguageType class_language, - std::vector &base_classes, + std::vector> &base_classes, std::vector &member_accessibilities, DWARFDIECollection &member_function_dies, DelayedPropertyList &delayed_properties, AccessType &default_accessibility, @@ -3271,9 +3266,14 @@ if (class_language == eLanguageTypeObjC) { ast->SetObjCSuperClass(class_clang_type, base_class_clang_type); } else { - base_classes.push_back(ast->CreateBaseClassSpecifier( - base_class_clang_type.GetOpaqueQualType(), accessibility, - is_virtual, is_base_of_class)); + std::unique_ptr result = + ast->CreateBaseClassSpecifier( + base_class_clang_type.GetOpaqueQualType(), accessibility, + is_virtual, is_base_of_class); + if (!result) + break; + + base_classes.push_back(std::move(result)); if (is_virtual) { // Do not specify any offset for virtual inheritance. The DWARF Index: source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h =================================================================== --- source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h +++ source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h @@ -40,7 +40,7 @@ CompilerType &m_derived_ct; clang::TagDecl &m_tag_decl; SymbolFileNativePDB &m_symbol_file; - std::vector m_bases; + std::vector> m_bases; ClangASTImporter::LayoutInfo m_layout; public: Index: source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp =================================================================== --- source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp +++ source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp @@ -59,12 +59,12 @@ CVType udt_cvt = m_symbol_file.m_index->tpi().getType(ti); lldb::opaque_compiler_type_t base_qt = base_ct.GetOpaqueQualType(); - clang::CXXBaseSpecifier *base_spec = + std::unique_ptr base_spec = m_symbol_file.GetASTContext().CreateBaseClassSpecifier( base_qt, TranslateMemberAccess(access), false, udt_cvt.kind() == LF_CLASS); - - m_bases.push_back(base_spec); + lldbassert(base_spec); + m_bases.push_back(std::move(base_spec)); return base_qt; } @@ -172,9 +172,8 @@ void UdtRecordCompleter::complete() { ClangASTContext &clang = m_symbol_file.GetASTContext(); - clang.SetBaseClassesForClassType(m_derived_ct.GetOpaqueQualType(), - m_bases.data(), m_bases.size()); - ClangASTContext::DeleteBaseClassSpecifiers(m_bases.data(), m_bases.size()); + clang.TransferBaseClasses(m_derived_ct.GetOpaqueQualType(), + std::move(m_bases)); clang.AddMethodOverridesForCXXRecordType(m_derived_ct.GetOpaqueQualType()); ClangASTContext::BuildIndirectFields(m_derived_ct); Index: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp =================================================================== --- source/Plugins/SymbolFile/PDB/PDBASTParser.cpp +++ source/Plugins/SymbolFile/PDB/PDBASTParser.cpp @@ -1208,7 +1208,8 @@ lldb_private::CompilerType &record_type, int record_kind, PDBBaseClassSymbolEnumerator &bases_enum, lldb_private::ClangASTImporter::LayoutInfo &layout_info) const { - std::vector base_classes; + std::vector> base_classes; + while (auto base = bases_enum.getNext()) { auto base_type = symbol_file.ResolveTypeUID(base->getTypeId()); if (!base_type) @@ -1229,13 +1230,13 @@ auto is_virtual = base->isVirtualBaseClass(); - auto base_class_spec = m_ast.CreateBaseClassSpecifier( - base_comp_type.GetOpaqueQualType(), access, is_virtual, - record_kind == clang::TTK_Class); - if (!base_class_spec) - continue; + std::unique_ptr base_spec = + m_ast.CreateBaseClassSpecifier(base_comp_type.GetOpaqueQualType(), + access, is_virtual, + record_kind == clang::TTK_Class); + lldbassert(base_spec); - base_classes.push_back(base_class_spec); + base_classes.push_back(std::move(base_spec)); if (is_virtual) continue; @@ -1247,13 +1248,9 @@ auto offset = clang::CharUnits::fromQuantity(base->getOffset()); layout_info.base_offsets.insert(std::make_pair(decl, offset)); } - if (!base_classes.empty()) { - m_ast.SetBaseClassesForClassType(record_type.GetOpaqueQualType(), - &base_classes.front(), - base_classes.size()); - ClangASTContext::DeleteBaseClassSpecifiers(&base_classes.front(), - base_classes.size()); - } + + m_ast.TransferBaseClasses(record_type.GetOpaqueQualType(), + std::move(base_classes)); } void PDBASTParser::AddRecordMethods(lldb_private::SymbolFile &symbol_file, Index: source/Symbol/ClangASTContext.cpp =================================================================== --- source/Symbol/ClangASTContext.cpp +++ source/Symbol/ClangASTContext.cpp @@ -8239,39 +8239,37 @@ #pragma mark C++ Base Classes -clang::CXXBaseSpecifier * +std::unique_ptr ClangASTContext::CreateBaseClassSpecifier(lldb::opaque_compiler_type_t type, AccessType access, bool is_virtual, bool base_of_class) { - if (type) - return new clang::CXXBaseSpecifier( - clang::SourceRange(), is_virtual, base_of_class, - ClangASTContext::ConvertAccessTypeToAccessSpecifier(access), - getASTContext()->getTrivialTypeSourceInfo(GetQualType(type)), - clang::SourceLocation()); - return nullptr; -} + if (!type) + return nullptr; -void ClangASTContext::DeleteBaseClassSpecifiers( - clang::CXXBaseSpecifier **base_classes, unsigned num_base_classes) { - for (unsigned i = 0; i < num_base_classes; ++i) { - delete base_classes[i]; - base_classes[i] = nullptr; - } + return llvm::make_unique( + clang::SourceRange(), is_virtual, base_of_class, + ClangASTContext::ConvertAccessTypeToAccessSpecifier(access), + getASTContext()->getTrivialTypeSourceInfo(GetQualType(type)), + clang::SourceLocation()); } -bool ClangASTContext::SetBaseClassesForClassType( +bool ClangASTContext::TransferBaseClasses( lldb::opaque_compiler_type_t type, - clang::CXXBaseSpecifier const *const *base_classes, - unsigned num_base_classes) { - if (type) { - clang::CXXRecordDecl *cxx_record_decl = GetAsCXXRecordDecl(type); - if (cxx_record_decl) { - cxx_record_decl->setBases(base_classes, num_base_classes); - return true; - } - } - return false; + std::vector> bases) { + if (!type) + return false; + clang::CXXRecordDecl *cxx_record_decl = GetAsCXXRecordDecl(type); + if (!cxx_record_decl) + return false; + std::vector raw_bases; + raw_bases.reserve(bases.size()); + + // Clang will make a copy of them, so it's ok that we pass pointers that we're + // about to destroy. + for (auto &b : bases) + raw_bases.push_back(b.get()); + cxx_record_decl->setBases(raw_bases.data(), raw_bases.size()); + return true; } bool ClangASTContext::SetObjCSuperClass( Index: unittests/Symbol/TestClangASTContext.cpp =================================================================== --- unittests/Symbol/TestClangASTContext.cpp +++ unittests/Symbol/TestClangASTContext.cpp @@ -337,15 +337,19 @@ EXPECT_NE(nullptr, non_empty_base_field_decl); EXPECT_TRUE(ClangASTContext::RecordHasFields(non_empty_base_decl)); + std::vector> bases; + // Test that a record with no direct fields, but fields in a base returns true CompilerType empty_derived = m_ast->CreateRecordType( nullptr, lldb::eAccessPublic, "EmptyDerived", clang::TTK_Struct, lldb::eLanguageTypeC_plus_plus, nullptr); ClangASTContext::StartTagDeclarationDefinition(empty_derived); - CXXBaseSpecifier *non_empty_base_spec = m_ast->CreateBaseClassSpecifier( - non_empty_base.GetOpaqueQualType(), lldb::eAccessPublic, false, false); - bool result = m_ast->SetBaseClassesForClassType( - empty_derived.GetOpaqueQualType(), &non_empty_base_spec, 1); + std::unique_ptr non_empty_base_spec = + m_ast->CreateBaseClassSpecifier(non_empty_base.GetOpaqueQualType(), + lldb::eAccessPublic, false, false); + bases.push_back(std::move(non_empty_base_spec)); + bool result = m_ast->TransferBaseClasses(empty_derived.GetOpaqueQualType(), + std::move(bases)); ClangASTContext::CompleteTagDeclarationDefinition(empty_derived); EXPECT_TRUE(result); CXXRecordDecl *empty_derived_non_empty_base_cxx_decl = @@ -363,10 +367,12 @@ nullptr, lldb::eAccessPublic, "EmptyDerived2", clang::TTK_Struct, lldb::eLanguageTypeC_plus_plus, nullptr); ClangASTContext::StartTagDeclarationDefinition(empty_derived2); - CXXBaseSpecifier *non_empty_vbase_spec = m_ast->CreateBaseClassSpecifier( - non_empty_base.GetOpaqueQualType(), lldb::eAccessPublic, true, false); - result = m_ast->SetBaseClassesForClassType(empty_derived2.GetOpaqueQualType(), - &non_empty_vbase_spec, 1); + std::unique_ptr non_empty_vbase_spec = + m_ast->CreateBaseClassSpecifier(non_empty_base.GetOpaqueQualType(), + lldb::eAccessPublic, true, false); + bases.push_back(std::move(non_empty_vbase_spec)); + result = m_ast->TransferBaseClasses(empty_derived2.GetOpaqueQualType(), + std::move(bases)); ClangASTContext::CompleteTagDeclarationDefinition(empty_derived2); EXPECT_TRUE(result); CXXRecordDecl *empty_derived_non_empty_vbase_cxx_decl = @@ -377,9 +383,6 @@ empty_derived_non_empty_vbase_cxx_decl, false)); EXPECT_TRUE( ClangASTContext::RecordHasFields(empty_derived_non_empty_vbase_decl)); - - delete non_empty_base_spec; - delete non_empty_vbase_spec; } TEST_F(TestClangASTContext, TemplateArguments) {