labath added a comment.
LGTM, modulo the defensive check. Incidentally, I was just looking at a
DW_AT_const_value bug, but I think it's a different issue than this one.
================
Comment at: lldb/source/Core/ValueObjectVariable.cpp:136
+ if (expr.GetExpressionData(m_data)) {
+ if (m_data.GetDataStart() && m_data.GetByteSize())
+ m_value.SetBytes(m_data.GetDataStart(), m_data.GetByteSize());
----------------
I doubt this check is necessary -- surely we should be able to rely on
`GetExpressionData` setting the data extractor to something reasonable if it
returns success.
If anything, it would be nice to make a test with an empty DW_AT_const_value,
which checks that lldb does something reasonable. It shouldn't that hard --
copy the DW_TAG_variable abbreviation, change DW_AT_const_value to
DW_FORM_block, and make a new variable DIE with an empty block.
================
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
----------------
aprantl wrote:
> This sentence is complicated to parse.
It may be enough to say "Check that we're able to display variables whose value
is given by DW_AT_const_value" -- the relationship between (or even existance)
of ValueObjectVariable and DWARFExpression may change, but the test case is
sufficiently generic to remain valid. Though, given the test name, even that
sentence is pretty redundant.
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
