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

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to