[lldb][DWARFASTParserClang][NFCI] Simplify ParseChildParameters

[Michael137]

This patch refactors ParseChildParameters in a way which makes it (in my opinion) more readable, removing some redundant local variables in the process and reduces the scope of some variables.

Motivation

Since DW_AT_object_pointers are now attached to declarations, we can test for their existence to check whether a C++ method is static or not (whereas currently we're deducing this from ParseChildParameters based on some heuristics we know are true for most compilers). So my plan is to move the code for determining type_quals and is_static out of ParseChildParameters. The refactoring in this PR will make this follow-up patch hopefully easier to review.

Testing

• This should be NFC. The main change is that we now no longer iterate over GetAttributes() but instead retrieve the name, type and is_artificial attributes of the parameters individually.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)

Changes

This patch refactors ParseChildParameters in a way which makes it (in my opinion) more readable, removing some redundant local variables in the process and reduces the scope of some variables.

Motivation

Since DW_AT_object_pointers are now attached to declarations, we can make test for their existence to check whether a C++ method is static or not (whereas currently we're deducing this from ParseChildParameters based on some heuristics we know are true for most compilers). So my plan is to move the code for determining type_quals and is_static out of ParseChildParameters. The refactoring in this PR will make this follow-up patch hopefully easier to review.

Testing

• This should be NFC. The main change is that we now no longer iterate over GetAttributes() but instead retrieve the name, type and is_artificial attributes of the parameters individually.

---

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

1 Files Affected:

• (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp (+7-51)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 68fa1d13943a16..4df95513f8dfa2 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -3084,50 +3084,11 @@ size_t DWARFASTParserClang::ParseChildParameters(
     const dw_tag_t tag = die.Tag();
     switch (tag) {
     case DW_TAG_formal_parameter: {
-      DWARFAttributes attributes = die.GetAttributes();
-      if (attributes.Size() == 0) {
-        arg_idx++;
-        break;
-      }
-
-      const char *name = nullptr;
-      DWARFFormValue param_type_die_form;
-      bool is_artificial = false;
-      // one of None, Auto, Register, Extern, Static, PrivateExtern
-
-      clang::StorageClass storage = clang::SC_None;
-      uint32_t i;
-      for (i = 0; i < attributes.Size(); ++i) {
-        const dw_attr_t attr = attributes.AttributeAtIndex(i);
-        DWARFFormValue form_value;
-        if (attributes.ExtractFormValueAtIndex(i, form_value)) {
-          switch (attr) {
-          case DW_AT_name:
-            name = form_value.AsCString();
-            break;
-          case DW_AT_type:
-            param_type_die_form = form_value;
-            break;
-          case DW_AT_artificial:
-            is_artificial = form_value.Boolean();
-            break;
-          case DW_AT_location:
-          case DW_AT_const_value:
-          case DW_AT_default_value:
-          case DW_AT_description:
-          case DW_AT_endianity:
-          case DW_AT_is_optional:
-          case DW_AT_segment:
-          case DW_AT_variable_parameter:
-          default:
-          case DW_AT_abstract_origin:
-          case DW_AT_sibling:
-            break;
-          }
-        }
-      }
+      arg_idx++;
+      const char *name = die.GetName();
+      DWARFDIE param_type_die = die.GetAttributeValueAsReferenceDIE(DW_AT_type);
 
-      if (is_artificial) {
+      if (die.GetAttributeValueAsUnsigned(DW_AT_artificial, 0)) {
         // In order to determine if a C++ member function is "const" we
         // have to look at the const-ness of "this"...
         if (arg_idx == 0 &&
@@ -3136,8 +3097,7 @@ size_t DWARFASTParserClang::ParseChildParameters(
             // specification DIEs, so we can't rely upon the name being in
             // the formal parameter DIE...
             (name == nullptr || ::strcmp(name, "this") == 0)) {
-          Type *this_type = die.ResolveTypeUID(param_type_die_form.Reference());
-          if (this_type) {
+          if (Type *this_type = die.ResolveTypeUID(param_type_die)) {
             uint32_t encoding_mask = this_type->GetEncodingMask();
             if (encoding_mask & Type::eEncodingIsPointerUID) {
               is_static = false;
@@ -3149,21 +3109,17 @@ size_t DWARFASTParserClang::ParseChildParameters(
             }
           }
         }
-      } else {
-        Type *type = die.ResolveTypeUID(param_type_die_form.Reference());
-        if (type) {
+      } else if (Type *type = die.ResolveTypeUID(param_type_die)) {
           function_param_types.push_back(type->GetForwardCompilerType());
 
           clang::ParmVarDecl *param_var_decl = m_ast.CreateParameterDeclaration(
               containing_decl_ctx, GetOwningClangModule(die), name,
-              type->GetForwardCompilerType(), storage);
+              type->GetForwardCompilerType(), clang::StorageClass::SC_None);
           assert(param_var_decl);
           function_param_decls.push_back(param_var_decl);
 
           m_ast.SetMetadataAsUserID(param_var_decl, die.GetID());
-        }
       }
-      arg_idx++;
     } break;
 
     case DW_TAG_unspecified_parameters:

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.