[PATCH] D42468: [lldb][PPC64] Fixed vector and struct return value

[Pavel Labath via Phabricator via llvm-commits]

labath added inline comments.


================
Comment at: source/Plugins/ABI/SysV-ppc64/ABISysV_ppc64.cpp:393-421
+#define LOG_PREFIX "ReturnValueExtractor: "
 
-  RegisterContext *reg_ctx = thread.GetRegisterContext().get();
-  if (!reg_ctx)
-    return return_valobj_sp;
+#define LOG(...) \
+  m_logger.LogMsg(LOG_PREFIX __VA_ARGS__)
 
-  const uint32_t type_flags = return_compiler_type.GetTypeInfo();
-  if (type_flags & eTypeIsScalar) {
-    value.SetValueType(Value::eValueTypeScalar);
+class ReturnValueExtractor {
+  // Log wrapper
----------------
The `LLDB_LOG` macro might be of interest to you. It will not be *as* concise as your solution here, but it should get you most of the benefits without having to reinvent your own logging infrastructure.


================
Comment at: source/Plugins/ABI/SysV-ppc64/ABISysV_ppc64.cpp:551-579
+  // constructor - can't fail
+  ReturnValueExtractor(Thread &thread, CompilerType &type)
+    : m_thread(thread),
+      m_type(type) {
+    m_byte_size = m_type.GetByteSize(nullptr);
+    m_data_ap.reset(new DataBufferHeap(m_byte_size, 0));
+  }
----------------
Instead of the constructor+init combo you can use a static factory function which returns `llvm::Expected<ReturnValueExtractor>`. The factory function can do all the operations that can fail, and then construct the object using a simple (possibly private) constructor. That way you never end up having a partially initialized object.


================
Comment at: source/Plugins/ABI/SysV-ppc64/ABISysV_ppc64.cpp:709
+      case sizeof(float):
+        value_sp->GetScalar() = (float)de.GetDouble(&offset);
         break;
----------------
Should this really be GetDouble?


Repository:
  rL LLVM

https://reviews.llvm.org/D42468