[lldb][NFC] Let CompilerType::DumpTypeValue take a option struct instead of a huge list of options

[teemperor]

Summary

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.

[labath]

I can't say I'm a fan of one-shot argument pack structures... For instance, one thing that's lost by this particular implementation is the implicit "nonnull" annotations on the arguments conferred by the references (the data extractor is already "nonnull", the stream isn't but it probably ought to be).

IIRC, we have a nontrivial number of functions which have a need to take a "reference" to some bitfield-y thing. For instance, the DumpDataExtractor function, which is also mentioned in this patch... Can we come up with some clever way to pack the arguments specifying the data to process into a single entity? That would replace five of these arguments by one, which should bring down the total number of arguments to this function to a reasonable size. And it would be usable in other contexts too...

[teemperor]

D70294 is the new thing

lldb/include/lldb/Symbol/ClangASTContext.h

#endif
 
   void DumpValue(lldb::opaque_compiler_type_t type, ExecutionContext *exe_ctx,
                  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,
                  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::offset_t data_offset, size_t data_byte_size) override;
 
   void DumpTypeDescription(
       lldb::opaque_compiler_type_t type) override; // Dump to stdout
 

lldb/include/lldb/Symbol/CompilerType.h

 #endif
 
   void DumpValue(ExecutionContext *exe_ctx, 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, 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,
                    size_t data_byte_size);
 
   void DumpTypeDescription() const; // Dump to stdout
 
   void DumpTypeDescription(Stream *s) const;

lldb/include/lldb/Symbol/TypeSystem.h

#endif
   virtual void DumpValue(lldb::opaque_compiler_type_t type,
                          ExecutionContext *exe_ctx, 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, 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
 
   virtual void DumpTypeDescription(lldb::opaque_compiler_type_t type,
                                    Stream *s) = 0;
 
   // TODO: These methods appear unused. Should they be removed?

lldb/source/DataFormatters/TypeFormat.cpp

if (context_type == Value::eContextTypeRegisterInfo) {
         }
 
         ExecutionContextScope *exe_scope =
             exe_ctx.GetBestExecutionContextScope();
         llvm::Optional<uint64_t> size = compiler_type.GetByteSize(exe_scope);
         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
         // here, but that's about as severe as we get
         // CompilerType::DumpTypeValue() should always return something, even
         // if that something is an error message
         dest = sstr.GetString();
       }
if (!valobj_enum_type.IsValid())
     return false;
   DataExtractor data;
   Status error;
   valobj->GetData(data, error);
   if (error.Fail())
     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();
 }
 
 std::string TypeFormatImpl_EnumType::GetDescription() {
   StreamString sstr;
   sstr.Printf("as type %s%s%s%s", m_enum_type.AsCString("<invalid type>"),
               Cascades() ? "" : " (not cascading)",
               SkipsPointers() ? " (skip pointers)" : "",
               SkipsReferences() ? " (skip references)" : "");
   return sstr.GetString();
 }

lldb/source/Symbol/ClangASTContext.cpp

static bool DumpEnumValue(const clang::QualType &qual_type, Stream *s,
 
   // If there is a remainder that is not covered by the value, print it as hex.
   if (remaining_value)
     s->Printf("0x%" PRIx64, remaining_value);
 
   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)) {
     return false;
   } else {
     clang::QualType qual_type(GetQualType(type));
 
     const clang::Type::TypeClass type_class = qual_type->getTypeClass();
 
     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) {
     case clang::Type::Typedef: {
       clang::QualType typedef_qual_type =
           llvm::cast<clang::TypedefType>(qual_type)
               ->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;
 
     default:
       // We are down to a scalar type that we just need to display.
       {
         uint32_t item_count = 1;
         // 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:
         case eFormatComplex:
         case eFormatCString: // NULL terminated C strings
         case eFormatDecimal:
         case eFormatEnum:
         case eFormatHex:
default:
         case eFormatVectorOfUInt128:
           break;
 
         case eFormatChar:
         case eFormatCharPrintable:
         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;
     }
   }
   return false;
 }
 
 void ClangASTContext::DumpSummary(lldb::opaque_compiler_type_t type,

lldb/source/Symbol/CompilerType.cpp

void CompilerType::DumpValue(ExecutionContext *exe_ctx, Stream *s,
   if (!IsValid())
     return;
   m_type_system->DumpValue(m_type, exe_ctx, s, format, data, data_byte_offset,
                            data_byte_size, bitfield_bit_size,
                            bitfield_bit_offset, show_types, show_summary,
                            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,
                                const DataExtractor &data,
                                lldb::offset_t data_byte_offset,
                                size_t data_byte_size) {
   if (IsValid())
     m_type_system->DumpSummary(m_type, exe_ctx, s, data, data_byte_offset,