[Lldb-commits] [lldb] [lldb][DWARFASTParserClang][NFCI] Simplify ParseChildParameters (PR #123790)

[via lldb-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)

<details>
<summary>Changes</summary>

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_pointer`s 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
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:

``````````

</details>


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