[Lldb-commits] [PATCH] D86311: Fix how ValueObjectVariable handles DW_AT_const_value when the DWARFExpression holds the data that represents a constant value

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

The code looks fine, I think the test needs an extra REQUIRES.



================
Comment at: lldb/source/Core/ValueObjectVariable.cpp:137
+       if (m_data.GetDataStart() && m_data.GetByteSize())
+        m_value.SetBytes(m_data.GetDataStart(), m_data.GetByteSize());
       m_value.SetContext(Value::eContextTypeVariable, variable);
----------------
I guess this looks reasonable.


================
Comment at: lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s:2
+# RUN: llvm-mc -filetype=obj -o %t -triple x86_64-apple-macosx10.15.0 %s
+# RUN: %lldb %t -o "target variable constant" -b | FileCheck %s
+
----------------
I think this is missing a REQUIRES: line that checks for an x86 target?


================
Comment at: lldb/test/Shell/SymbolFile/DWARF/DW_AT_const_value.s:11
+# This is testing when how ValueObjectVariable handles the case where the
+# DWARFExpression holds the data that represents a constant value.
+# Compile at -O1 allows us to capture this case. Below is the code used
----------------
This sentence is complicated to parse.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86311/new/

https://reviews.llvm.org/D86311