[Lldb-commits] [PATCH] D125509: [LLDB][NFC] Decouple dwarf location table from DWARFExpression.

Jonas Devlieghere via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Wed May 18 17:11:36 PDT 2022

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++

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 {
+  // 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
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?

  rG LLVM Github Monorepo



More information about the lldb-commits mailing list