[Lldb-commits] [lldb] 12a89e1 - [lldb][NFCI] Remove redundant accessibility heuristic in the DWARF parser
Raphael Isemann via lldb-commits
lldb-commits at lists.llvm.org
Thu Jul 22 04:36:44 PDT 2021
Author: Raphael Isemann
Date: 2021-07-22T13:36:23+02:00
New Revision: 12a89e14b83ac3db9e44f535a43bb11e7b6c3601
URL: https://github.com/llvm/llvm-project/commit/12a89e14b83ac3db9e44f535a43bb11e7b6c3601
DIFF: https://github.com/llvm/llvm-project/commit/12a89e14b83ac3db9e44f535a43bb11e7b6c3601.diff
LOG: [lldb][NFCI] Remove redundant accessibility heuristic in the DWARF parser
LLDB's DWARF parser has some heuristics for guessing and fixing up the
accessibility of C++ class/struct members after they were already created in the
internal Clang AST. The heuristic is that if a struct/class has a base class,
then it's actually a class and it's members are private unless otherwise
specified.
>From what I can see this heuristic isn't sound and also unnecessary. The idea
that inheritance implies that the `class` keyword was used and the default
visibility is `private` is incorrect. Also both GCC and Clang use
`DW_TAG_structure_type` and `DW_TAG_class_type` for `struct` and `class` types
respectively, so the default visibility we infer from that information is always
correct and there is no need to fix it up.
And finally, the access specifiers we set in the Clang AST are anyway unused
within LLDB. The expression parser explicitly ignores them to give users access
to private members and there is not SBAPI functionality that exposes this
information.
This patch removes all the heuristic code for the reasons above and instead
just relies on the access values we infer from the tag kind and explicit
annotations in DWARF.
This patch is NFCI.
Reviewed By: werat
Differential Revision: https://reviews.llvm.org/D105463
Added:
Modified:
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
Removed:
################################################################################
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index f10fbceaea540..4cf4ea6ed0f4c 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1983,15 +1983,12 @@ bool DWARFASTParserClang::CompleteRecordType(const DWARFDIE &die,
}
std::vector<std::unique_ptr<clang::CXXBaseSpecifier>> bases;
- std::vector<int> member_accessibilities;
- bool is_a_class = false;
// Parse members and base classes first
std::vector<DWARFDIE> member_function_dies;
DelayedPropertyList delayed_properties;
- ParseChildMembers(die, clang_type, bases, member_accessibilities,
- member_function_dies, delayed_properties,
- default_accessibility, is_a_class, layout_info);
+ ParseChildMembers(die, clang_type, bases, member_function_dies,
+ delayed_properties, default_accessibility, layout_info);
// Now parse any methods if there were any...
for (const DWARFDIE &die : member_function_dies)
@@ -2012,31 +2009,6 @@ bool DWARFASTParserClang::CompleteRecordType(const DWARFDIE &die,
}
}
- // If we have a DW_TAG_structure_type instead of a DW_TAG_class_type we
- // need to tell the clang type it is actually a class.
- if (!type_is_objc_object_or_interface) {
- if (is_a_class && tag_decl_kind != clang::TTK_Class)
- m_ast.SetTagTypeKind(ClangUtil::GetQualType(clang_type),
- clang::TTK_Class);
- }
-
- // Since DW_TAG_structure_type gets used for both classes and
- // structures, we may need to set any DW_TAG_member fields to have a
- // "private" access if none was specified. When we parsed the child
- // members we tracked that actual accessibility value for each
- // DW_TAG_member in the "member_accessibilities" array. If the value
- // for the member is zero, then it was set to the
- // "default_accessibility" which for structs was "public". Below we
- // correct this by setting any fields to "private" that weren't
- // correctly set.
- if (is_a_class && !member_accessibilities.empty()) {
- // This is a class and all members that didn't have their access
- // specified are private.
- m_ast.SetDefaultAccessForRecordFields(
- m_ast.GetAsRecordDecl(clang_type), eAccessPrivate,
- &member_accessibilities.front(), member_accessibilities.size());
- }
-
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
@@ -2349,7 +2321,6 @@ Function *DWARFASTParserClang::ParseFunctionFromDWARF(CompileUnit &comp_unit,
void DWARFASTParserClang::ParseSingleMember(
const DWARFDIE &die, const DWARFDIE &parent_die,
const lldb_private::CompilerType &class_clang_type,
- std::vector<int> &member_accessibilities,
lldb::AccessType default_accessibility,
DelayedPropertyList &delayed_properties,
lldb_private::ClangASTImporter::LayoutInfo &layout_info,
@@ -2557,7 +2528,6 @@ void DWARFASTParserClang::ParseSingleMember(
if (accessibility == eAccessNone)
accessibility = default_accessibility;
- member_accessibilities.push_back(accessibility);
uint64_t field_bit_offset =
(member_byte_offset == UINT32_MAX ? 0 : (member_byte_offset * 8));
@@ -2767,10 +2737,9 @@ void DWARFASTParserClang::ParseSingleMember(
bool DWARFASTParserClang::ParseChildMembers(
const DWARFDIE &parent_die, CompilerType &class_clang_type,
std::vector<std::unique_ptr<clang::CXXBaseSpecifier>> &base_classes,
- std::vector<int> &member_accessibilities,
std::vector<DWARFDIE> &member_function_dies,
DelayedPropertyList &delayed_properties, AccessType &default_accessibility,
- bool &is_a_class, ClangASTImporter::LayoutInfo &layout_info) {
+ ClangASTImporter::LayoutInfo &layout_info) {
if (!parent_die)
return false;
@@ -2790,8 +2759,8 @@ bool DWARFASTParserClang::ParseChildMembers(
case DW_TAG_member:
case DW_TAG_APPLE_property:
ParseSingleMember(die, parent_die, class_clang_type,
- member_accessibilities, default_accessibility,
- delayed_properties, layout_info, last_field_info);
+ default_accessibility, delayed_properties, layout_info,
+ last_field_info);
break;
case DW_TAG_subprogram:
@@ -2800,9 +2769,6 @@ bool DWARFASTParserClang::ParseChildMembers(
break;
case DW_TAG_inheritance: {
- is_a_class = true;
- if (default_accessibility == eAccessNone)
- default_accessibility = eAccessPrivate;
// TODO: implement DW_TAG_inheritance type parsing
DWARFAttributes attributes;
const size_t num_attributes = die.GetAttributes(attributes);
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
index e13716b95c1cc..9bf6240b75544 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -111,10 +111,9 @@ class DWARFASTParserClang : public DWARFASTParser {
bool ParseChildMembers(
const DWARFDIE &die, lldb_private::CompilerType &class_compiler_type,
std::vector<std::unique_ptr<clang::CXXBaseSpecifier>> &base_classes,
- std::vector<int> &member_accessibilities,
std::vector<DWARFDIE> &member_function_dies,
DelayedPropertyList &delayed_properties,
- lldb::AccessType &default_accessibility, bool &is_a_class,
+ lldb::AccessType &default_accessibility,
lldb_private::ClangASTImporter::LayoutInfo &layout_info);
size_t
@@ -194,7 +193,6 @@ class DWARFASTParserClang : public DWARFASTParser {
void
ParseSingleMember(const DWARFDIE &die, const DWARFDIE &parent_die,
const lldb_private::CompilerType &class_clang_type,
- std::vector<int> &member_accessibilities,
lldb::AccessType default_accessibility,
DelayedPropertyList &delayed_properties,
lldb_private::ClangASTImporter::LayoutInfo &layout_info,
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index df21c00a61c5f..7150fdc784762 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -2575,42 +2575,6 @@ ClangASTMetadata *TypeSystemClang::GetMetadata(const clang::Type *object) {
return nullptr;
}
-bool TypeSystemClang::SetTagTypeKind(clang::QualType tag_qual_type,
- int kind) const {
- const clang::Type *clang_type = tag_qual_type.getTypePtr();
- if (clang_type) {
- const clang::TagType *tag_type = llvm::dyn_cast<clang::TagType>(clang_type);
- if (tag_type) {
- clang::TagDecl *tag_decl =
- llvm::dyn_cast<clang::TagDecl>(tag_type->getDecl());
- if (tag_decl) {
- tag_decl->setTagKind((clang::TagDecl::TagKind)kind);
- return true;
- }
- }
- }
- return false;
-}
-
-bool TypeSystemClang::SetDefaultAccessForRecordFields(
- clang::RecordDecl *record_decl, int default_accessibility,
- int *assigned_accessibilities, size_t num_assigned_accessibilities) {
- if (record_decl) {
- uint32_t field_idx;
- clang::RecordDecl::field_iterator field, field_end;
- for (field = record_decl->field_begin(),
- field_end = record_decl->field_end(), field_idx = 0;
- field != field_end; ++field, ++field_idx) {
- // If no accessibility was assigned, assign the correct one
- if (field_idx < num_assigned_accessibilities &&
- assigned_accessibilities[field_idx] == clang::AS_none)
- field->setAccess((clang::AccessSpecifier)default_accessibility);
- }
- return true;
- }
- return false;
-}
-
clang::DeclContext *
TypeSystemClang::GetDeclContextForType(const CompilerType &type) {
return GetDeclContextForType(ClangUtil::GetQualType(type));
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
index 62679e99d234d..701e4ca42e399 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -380,13 +380,6 @@ class TypeSystemClang : public TypeSystem {
bool isForwardDecl, bool isInternal,
ClangASTMetadata *metadata = nullptr);
- bool SetTagTypeKind(clang::QualType type, int kind) const;
-
- bool SetDefaultAccessForRecordFields(clang::RecordDecl *record_decl,
- int default_accessibility,
- int *assigned_accessibilities,
- size_t num_assigned_accessibilities);
-
// Returns a mask containing bits from the TypeSystemClang::eTypeXXX
// enumerations
More information about the lldb-commits
mailing list