JDevlieghere added a comment.
Separating the notion of an expression and an expression list seems worthwhile.
I did a very high level pass but I'll take a more detailed look tomorrow.
================
Comment at: lldb/include/lldb/Expression/DWARFExpressionList.h:1-2
+//===-- DWARFExpressionList.h ---------------------------------------*- C++
+//-*-===//
+//
----------------
Formatting
================
Comment at: lldb/include/lldb/Expression/DWARFExpressionList.h:22
+
+// It's the interface to get or evaluate a DWARFExpression.
+class DWARFExpressionList {
----------------
This should be a Doxygen comment. But more importantly, this doesn't really
tell me what the class does. DWARFExpressionList implies it holds a list of
expressions so that seems useful to include there.
================
Comment at: lldb/include/lldb/Expression/DWARFExpressionList.h:24
+class DWARFExpressionList {
+private:
+ // RangeDataVector requires a comparator for DWARFExpression, but it doesn't
----------------
I think we generally put public first, followed by protected and private last.
================
Comment at: lldb/test/Shell/SymbolFile/DWARF/x86/dwp.s:22-23
# SYMBOLS-NEXT: Block{{.*}}, ranges = [0x00000000-0x00000001)
-# SYMBOLS-NEXT: Variable{{.*}}, name = "x", {{.*}}, location =
-# SYMBOLS-NEXT: DW_LLE_startx_length (0x0000000000000001,
0x0000000000000001): DW_OP_reg0 RAX
+# SYMBOLS-NEXT: Variable{{.*}}, name = "x", {{.*}}, location =
+# SYMBOLS-NEXT: [0x0000000000000000, 0x0000000000000001): DW_OP_reg0 RAX
# SYMBOLS-EMPTY:
----------------
I guess the patch is not NFC is the output changes? Would it be possible to
split the functional and non-functional part of this patch into separate
patches?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125509/new/
https://reviews.llvm.org/D125509
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
