Skip to content

[lldb][DWARFASTParserClang] Make MemberAttributes const when parsing member DIEs #71921

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 13, 2023

Conversation

Michael137
Copy link
Member

This patch removes the Objective-C accessibility workaround added in 5a477cf (rdar://8492646). This allows us to make the local MemberAttributes variable immutable, which is useful for some other work around this function I was planning on doing.

We don't need the workaround anymore since compiler-support for giving debuggers access to private ivars was done couple of years later in d6cb4a8 (rdar://10997647).

Testing

  • Test-suite runs cleanly

@llvmbot
Copy link
Member

llvmbot commented Nov 10, 2023

@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)

Changes

This patch removes the Objective-C accessibility workaround added in 5a477cf (rdar://8492646). This allows us to make the local MemberAttributes variable immutable, which is useful for some other work around this function I was planning on doing.

We don't need the workaround anymore since compiler-support for giving debuggers access to private ivars was done couple of years later in d6cb4a8 (rdar://10997647).

Testing

  • Test-suite runs cleanly

Full diff: https://github.com/llvm/llvm-project/pull/71921.diff

1 Files Affected:

  • (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp (+15-21)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 3174c18c97d888c..4d87117cfe9c195 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2869,15 +2869,7 @@ void DWARFASTParserClang::ParseSingleMember(
   const uint64_t parent_bit_size =
       parent_byte_size == UINT64_MAX ? UINT64_MAX : parent_byte_size * 8;
 
-  // FIXME: Remove the workarounds below and make this const.
-  MemberAttributes attrs(die, parent_die, module_sp);
-
-  const bool class_is_objc_object_or_interface =
-      TypeSystemClang::IsObjCObjectOrInterfaceType(class_clang_type);
-
-  // FIXME: Make Clang ignore Objective-C accessibility for expressions
-  if (class_is_objc_object_or_interface)
-    attrs.accessibility = eAccessNone;
+  const MemberAttributes attrs(die, parent_die, module_sp);
 
   // Handle static members, which are typically members without
   // locations. However, GCC doesn't emit DW_AT_data_member_location
@@ -2892,13 +2884,13 @@ void DWARFASTParserClang::ParseSingleMember(
   if (attrs.member_byte_offset == UINT32_MAX &&
       attrs.data_bit_offset == UINT64_MAX && attrs.is_declaration) {
     Type *var_type = die.ResolveTypeUID(attrs.encoding_form.Reference());
-
     if (var_type) {
-      if (attrs.accessibility == eAccessNone)
-        attrs.accessibility = eAccessPublic;
+      const auto accessibility = attrs.accessibility == eAccessNone
+                                     ? eAccessPublic
+                                     : attrs.accessibility;
       CompilerType ct = var_type->GetForwardCompilerType();
       clang::VarDecl *v = TypeSystemClang::AddVariableToRecordType(
-          class_clang_type, attrs.name, ct, attrs.accessibility);
+          class_clang_type, attrs.name, ct, accessibility);
       if (!v) {
         LLDB_LOG(log, "Failed to add variable to the record type");
         return;
@@ -2942,8 +2934,9 @@ void DWARFASTParserClang::ParseSingleMember(
   const uint64_t word_width = 32;
   CompilerType member_clang_type = member_type->GetLayoutCompilerType();
 
-  if (attrs.accessibility == eAccessNone)
-    attrs.accessibility = default_accessibility;
+  const auto accessibility = attrs.accessibility == eAccessNone
+                                 ? default_accessibility
+                                 : attrs.accessibility;
 
   uint64_t field_bit_offset = (attrs.member_byte_offset == UINT32_MAX
                                    ? 0
@@ -2957,12 +2950,13 @@ void DWARFASTParserClang::ParseSingleMember(
     if (attrs.data_bit_offset != UINT64_MAX) {
       this_field_info.bit_offset = attrs.data_bit_offset;
     } else {
-      if (!attrs.byte_size)
-        attrs.byte_size = member_type->GetByteSize(nullptr);
+      auto byte_size = attrs.byte_size;
+      if (!byte_size)
+        byte_size = member_type->GetByteSize(nullptr);
 
       ObjectFile *objfile = die.GetDWARF()->GetObjectFile();
       if (objfile->GetByteOrder() == eByteOrderLittle) {
-        this_field_info.bit_offset += attrs.byte_size.value_or(0) * 8;
+        this_field_info.bit_offset += byte_size.value_or(0) * 8;
         this_field_info.bit_offset -= (attrs.bit_offset + attrs.bit_size);
       } else {
         this_field_info.bit_offset += attrs.bit_offset;
@@ -3001,7 +2995,7 @@ void DWARFASTParserClang::ParseSingleMember(
     // unnamed bitfields if we have a new enough clang.
     bool detect_unnamed_bitfields = true;
 
-    if (class_is_objc_object_or_interface)
+    if (TypeSystemClang::IsObjCObjectOrInterfaceType(class_clang_type))
       detect_unnamed_bitfields =
           die.GetCU()->Supports_unnamed_objc_bitfields();
 
@@ -3033,7 +3027,7 @@ void DWARFASTParserClang::ParseSingleMember(
                 class_clang_type, llvm::StringRef(),
                 m_ast.GetBuiltinTypeForEncodingAndBitSize(eEncodingSint,
                                                           word_width),
-                attrs.accessibility, unnamed_field_info->bit_size);
+                accessibility, unnamed_field_info->bit_size);
 
         layout_info.field_offsets.insert(std::make_pair(
             unnamed_bitfield_decl, unnamed_field_info->bit_offset));
@@ -3105,7 +3099,7 @@ void DWARFASTParserClang::ParseSingleMember(
   TypeSystemClang::RequireCompleteType(member_clang_type);
 
   clang::FieldDecl *field_decl = TypeSystemClang::AddFieldToRecordType(
-      class_clang_type, attrs.name, member_clang_type, attrs.accessibility,
+      class_clang_type, attrs.name, member_clang_type, accessibility,
       attrs.bit_size);
 
   m_ast.SetMetadataAsUserID(field_decl, die.GetID());

@Michael137 Michael137 merged commit 576f7cc into llvm:main Nov 13, 2023
@Michael137 Michael137 deleted the bugfix/lldb-const-member-atttrs branch November 13, 2023 06:11
@Michael137 Michael137 restored the bugfix/lldb-const-member-atttrs branch November 13, 2023 06:16
Michael137 added a commit that referenced this pull request Nov 13, 2023
…parsing member DIEs (#71921)"

This reverts commit 576f7cc.

Causes build bot failure because new code started depending
on the non-constness of the MemberAttributes.
Michael137 added a commit that referenced this pull request Nov 13, 2023
…parsing member DIEs (#71921)"

Changed the recently added `FindConstantOnVariableDefinition` to
not rely on MemberAttributes being non-const.

Original commit message:
"""
This patch removes the Objective-C accessibility workaround added in
5a477cf
(rdar://8492646). This allows us to make the local `MemberAttributes`
variable immutable, which is useful for some other work around this
function I was planning on doing.

We don't need the workaround anymore since compiler-support for giving
debuggers access to private ivars was done couple of years later in
d6cb4a8
(rdar://10997647).

**Testing**

* Test-suite runs cleanly
"""
zahiraam pushed a commit to zahiraam/llvm-project that referenced this pull request Nov 20, 2023
…member DIEs (llvm#71921)

This patch removes the Objective-C accessibility workaround added in
llvm@5a477cf
(rdar://8492646). This allows us to make the local `MemberAttributes`
variable immutable, which is useful for some other work around this
function I was planning on doing.

We don't need the workaround anymore since compiler-support for giving
debuggers access to private ivars was done couple of years later in
llvm@d6cb4a8
(rdar://10997647).

**Testing**

* Test-suite runs cleanly
zahiraam pushed a commit to zahiraam/llvm-project that referenced this pull request Nov 20, 2023
…parsing member DIEs (llvm#71921)"

This reverts commit 576f7cc.

Causes build bot failure because new code started depending
on the non-constness of the MemberAttributes.
zahiraam pushed a commit to zahiraam/llvm-project that referenced this pull request Nov 20, 2023
…parsing member DIEs (llvm#71921)"

Changed the recently added `FindConstantOnVariableDefinition` to
not rely on MemberAttributes being non-const.

Original commit message:
"""
This patch removes the Objective-C accessibility workaround added in
llvm@5a477cf
(rdar://8492646). This allows us to make the local `MemberAttributes`
variable immutable, which is useful for some other work around this
function I was planning on doing.

We don't need the workaround anymore since compiler-support for giving
debuggers access to private ivars was done couple of years later in
llvm@d6cb4a8
(rdar://10997647).

**Testing**

* Test-suite runs cleanly
"""
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants