teemperor created this revision.
teemperor added reviewers: labath, davide.
Herald added subscribers: lldb-commits, JDevlieghere.
Herald added a project: LLDB.

This function has 8 parameters which makes its function calls hard to read. It 
also
doesn't help that half the parameters are just integers of some form. What's 
even worse is that
we fiddle around with all these signatures/invocations downstream in swift-lldb 
just because we
add one parameter there.

Let's just pass a struct here so that we can add the default values in a 
sensible way and actually
have some descriptive name beside the values we pass as parameters when we call 
this function.
Also moves the hidden documentation from the one function call to the struct so 
that it becomes
more obvious what's going on here.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D70120

Files:
  lldb/include/lldb/Symbol/ClangASTContext.h
  lldb/include/lldb/Symbol/CompilerType.h
  lldb/include/lldb/Symbol/TypeSystem.h
  lldb/source/DataFormatters/TypeFormat.cpp
  lldb/source/Symbol/ClangASTContext.cpp
  lldb/source/Symbol/CompilerType.cpp

Index: lldb/source/Symbol/CompilerType.cpp
===================================================================
--- lldb/source/Symbol/CompilerType.cpp
+++ lldb/source/Symbol/CompilerType.cpp
@@ -731,17 +731,10 @@
                            verbose, depth);
 }
 
-bool CompilerType::DumpTypeValue(Stream *s, lldb::Format format,
-                                 const DataExtractor &data,
-                                 lldb::offset_t byte_offset, size_t byte_size,
-                                 uint32_t bitfield_bit_size,
-                                 uint32_t bitfield_bit_offset,
-                                 ExecutionContextScope *exe_scope) {
+bool CompilerType::DumpTypeValue(DumpTypeValueOpts opts) {
   if (!IsValid())
     return false;
-  return m_type_system->DumpTypeValue(m_type, s, format, data, byte_offset,
-                                      byte_size, bitfield_bit_size,
-                                      bitfield_bit_offset, exe_scope);
+  return m_type_system->DumpTypeValue(m_type, opts);
 }
 
 void CompilerType::DumpSummary(ExecutionContext *exe_ctx, Stream *s,
Index: lldb/source/Symbol/ClangASTContext.cpp
===================================================================
--- lldb/source/Symbol/ClangASTContext.cpp
+++ lldb/source/Symbol/ClangASTContext.cpp
@@ -9468,11 +9468,8 @@
   return true;
 }
 
-bool ClangASTContext::DumpTypeValue(
-    lldb::opaque_compiler_type_t type, Stream *s, lldb::Format format,
-    const DataExtractor &data, lldb::offset_t byte_offset, size_t byte_size,
-    uint32_t bitfield_bit_size, uint32_t bitfield_bit_offset,
-    ExecutionContextScope *exe_scope) {
+bool ClangASTContext::DumpTypeValue(lldb::opaque_compiler_type_t type,
+                                    CompilerType::DumpTypeValueOpts opts) {
   if (!type)
     return false;
   if (IsAggregateType(type)) {
@@ -9484,8 +9481,7 @@
 
     if (type_class == clang::Type::Elaborated) {
       qual_type = llvm::cast<clang::ElaboratedType>(qual_type)->getNamedType();
-      return DumpTypeValue(qual_type.getAsOpaquePtr(), s, format, data, byte_offset, byte_size,
-                           bitfield_bit_size, bitfield_bit_offset, exe_scope);
+      return DumpTypeValue(qual_type.getAsOpaquePtr(), opts);
     }
 
     switch (type_class) {
@@ -9495,32 +9491,24 @@
               ->getDecl()
               ->getUnderlyingType();
       CompilerType typedef_clang_type(this, typedef_qual_type.getAsOpaquePtr());
-      if (format == eFormatDefault)
-        format = typedef_clang_type.GetFormat();
+      if (opts.format == eFormatDefault)
+        opts.format = typedef_clang_type.GetFormat();
       clang::TypeInfo typedef_type_info =
           getASTContext()->getTypeInfo(typedef_qual_type);
       uint64_t typedef_byte_size = typedef_type_info.Width / 8;
 
-      return typedef_clang_type.DumpTypeValue(
-          s,
-          format,            // The format with which to display the element
-          data,              // Data buffer containing all bytes for this type
-          byte_offset,       // Offset into "data" where to grab value from
-          typedef_byte_size, // Size of this type in bytes
-          bitfield_bit_size, // Size in bits of a bitfield value, if zero don't
-                             // treat as a bitfield
-          bitfield_bit_offset, // Offset in bits of a bitfield value if
-                               // bitfield_bit_size != 0
-          exe_scope);
+      opts.data_byte_size = typedef_byte_size;
+      return typedef_clang_type.DumpTypeValue(opts);
     } break;
 
     case clang::Type::Enum:
       // If our format is enum or default, show the enumeration value as its
       // enumeration string value, else just display it as requested.
-      if ((format == eFormatEnum || format == eFormatDefault) &&
+      if ((opts.format == eFormatEnum || opts.format == eFormatDefault) &&
           GetCompleteType(type))
-        return DumpEnumValue(qual_type, s, data, byte_offset, byte_size,
-                             bitfield_bit_offset, bitfield_bit_size);
+        return DumpEnumValue(qual_type, opts.stream, *opts.data,
+                             opts.data_offset, opts.data_byte_size,
+                             opts.bitfield_bit_offset, opts.bitfield_bit_size);
       // format was not enum, just fall through and dump the value as
       // requested....
       LLVM_FALLTHROUGH;
@@ -9532,7 +9520,7 @@
         // A few formats, we might need to modify our size and count for
         // depending
         // on how we are trying to display the value...
-        switch (format) {
+        switch (opts.format) {
         default:
         case eFormatBoolean:
         case eFormatBinary:
@@ -9566,24 +9554,25 @@
         case eFormatCharArray:
         case eFormatBytes:
         case eFormatBytesWithASCII:
-          item_count = byte_size;
-          byte_size = 1;
+          item_count = opts.data_byte_size;
+          opts.data_byte_size = 1;
           break;
 
         case eFormatUnicode16:
-          item_count = byte_size / 2;
-          byte_size = 2;
+          item_count = opts.data_byte_size / 2;
+          opts.data_byte_size = 2;
           break;
 
         case eFormatUnicode32:
-          item_count = byte_size / 4;
-          byte_size = 4;
+          item_count = opts.data_byte_size / 4;
+          opts.data_byte_size = 4;
           break;
         }
-        return DumpDataExtractor(data, s, byte_offset, format, byte_size,
-                                 item_count, UINT32_MAX, LLDB_INVALID_ADDRESS,
-                                 bitfield_bit_size, bitfield_bit_offset,
-                                 exe_scope);
+
+        return DumpDataExtractor(
+            *opts.data, opts.stream, opts.data_offset, opts.format,
+            opts.data_byte_size, item_count, UINT32_MAX, LLDB_INVALID_ADDRESS,
+            opts.bitfield_bit_size, opts.bitfield_bit_offset, opts.exe_scope);
       }
       break;
     }
Index: lldb/source/DataFormatters/TypeFormat.cpp
===================================================================
--- lldb/source/DataFormatters/TypeFormat.cpp
+++ lldb/source/DataFormatters/TypeFormat.cpp
@@ -99,15 +99,15 @@
         if (!size)
           return false;
         StreamString sstr;
-        compiler_type.DumpTypeValue(
-            &sstr,                          // The stream to use for display
-            GetFormat(),                    // Format to display this type with
-            data,                           // Data to extract from
-            0,                              // Byte offset into "m_data"
-            *size,                          // Byte size of item in "m_data"
-            valobj->GetBitfieldBitSize(),   // Bitfield bit size
-            valobj->GetBitfieldBitOffset(), // Bitfield bit offset
-            exe_scope);
+        CompilerType::DumpTypeValueOpts o;
+        o.data = &data;
+        o.stream = &sstr;
+        o.format = GetFormat();
+        o.data_byte_size = *size;
+        o.bitfield_bit_size = valobj->GetBitfieldBitSize();
+        o.bitfield_bit_offset = valobj->GetBitfieldBitOffset();
+        o.exe_scope = exe_scope;
+        compiler_type.DumpTypeValue(o);
         // Given that we do not want to set the ValueObject's m_error for a
         // formatting error (or else we wouldn't be able to reformat until a
         // next update), an empty string is treated as a "false" return from
@@ -187,9 +187,13 @@
     return false;
   ExecutionContext exe_ctx(valobj->GetExecutionContextRef());
   StreamString sstr;
-  valobj_enum_type.DumpTypeValue(&sstr, lldb::eFormatEnum, data, 0,
-                                 data.GetByteSize(), 0, 0,
-                                 exe_ctx.GetBestExecutionContextScope());
+  CompilerType::DumpTypeValueOpts o;
+  o.data = &data;
+  o.stream = &sstr;
+  o.format = lldb::eFormatEnum;
+  o.data_byte_size = data.GetByteSize();
+  o.exe_scope = exe_ctx.GetBestExecutionContextScope();
+  valobj_enum_type.DumpTypeValue(o);
   if (!sstr.GetString().empty())
     dest = sstr.GetString();
   return !dest.empty();
Index: lldb/include/lldb/Symbol/TypeSystem.h
===================================================================
--- lldb/include/lldb/Symbol/TypeSystem.h
+++ lldb/include/lldb/Symbol/TypeSystem.h
@@ -375,12 +375,8 @@
                          uint32_t bitfield_bit_offset, bool show_types,
                          bool show_summary, bool verbose, uint32_t depth) = 0;
 
-  virtual bool DumpTypeValue(lldb::opaque_compiler_type_t type, Stream *s,
-                             lldb::Format format, const DataExtractor &data,
-                             lldb::offset_t data_offset, size_t data_byte_size,
-                             uint32_t bitfield_bit_size,
-                             uint32_t bitfield_bit_offset,
-                             ExecutionContextScope *exe_scope) = 0;
+  virtual bool DumpTypeValue(lldb::opaque_compiler_type_t type,
+                             CompilerType::DumpTypeValueOpts opts) = 0;
 
   virtual void
   DumpTypeDescription(lldb::opaque_compiler_type_t type) = 0; // Dump to stdout
Index: lldb/include/lldb/Symbol/CompilerType.h
===================================================================
--- lldb/include/lldb/Symbol/CompilerType.h
+++ lldb/include/lldb/Symbol/CompilerType.h
@@ -346,10 +346,24 @@
                  uint32_t bitfield_bit_offset, bool show_types,
                  bool show_summary, bool verbose, uint32_t depth);
 
-  bool DumpTypeValue(Stream *s, lldb::Format format, const DataExtractor &data,
-                     lldb::offset_t data_offset, size_t data_byte_size,
-                     uint32_t bitfield_bit_size, uint32_t bitfield_bit_offset,
-                     ExecutionContextScope *exe_scope);
+  struct DumpTypeValueOpts {
+    /// Data buffer containing all bytes for this type.
+    const DataExtractor *data = nullptr;
+    Stream *stream = nullptr;
+    /// The format with which to display the type.
+    lldb::Format format = lldb::Format::eFormatDefault;
+    /// Offset into "data" where to grab value from.
+    lldb::offset_t data_offset = 0U;
+    /// Size of this type in bytes.
+    size_t data_byte_size = 0U;
+    /// Size in bits of a bitfield value, if zero don't treat as a bitfield.
+    uint32_t bitfield_bit_size = 0U;
+    /// Offset in bits of a bitfield value if bitfield_bit_size != 0
+    uint32_t bitfield_bit_offset = 0U;
+    ExecutionContextScope *exe_scope = nullptr;
+  };
+
+  bool DumpTypeValue(DumpTypeValueOpts opts);
 
   void DumpSummary(ExecutionContext *exe_ctx, Stream *s,
                    const DataExtractor &data, lldb::offset_t data_offset,
Index: lldb/include/lldb/Symbol/ClangASTContext.h
===================================================================
--- lldb/include/lldb/Symbol/ClangASTContext.h
+++ lldb/include/lldb/Symbol/ClangASTContext.h
@@ -901,11 +901,8 @@
                  bool show_types, bool show_summary, bool verbose,
                  uint32_t depth) override;
 
-  bool DumpTypeValue(lldb::opaque_compiler_type_t type, Stream *s,
-                     lldb::Format format, const DataExtractor &data,
-                     lldb::offset_t data_offset, size_t data_byte_size,
-                     uint32_t bitfield_bit_size, uint32_t bitfield_bit_offset,
-                     ExecutionContextScope *exe_scope) override;
+  bool DumpTypeValue(lldb::opaque_compiler_type_t type,
+                     CompilerType::DumpTypeValueOpts opts) override;
 
   void DumpSummary(lldb::opaque_compiler_type_t type, ExecutionContext *exe_ctx,
                    Stream *s, const DataExtractor &data,
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to