wallace requested changes to this revision.
wallace added a comment.
This revision now requires changes to proceed.
Much better. Just some better patterns that you could follow. Read my comments.
Besides that, now you can unify the tests across libcxx and libstdcpp
================
Comment at: lldb/examples/synthetic/gnu_libstdcpp.py:13-15
+ def __init__(self, valobj, dict, has_prev, node_value_pointer_offset):
logger = lldb.formatters.Logger.Logger()
self.valobj = valobj
----------------
Add these comments and remove the node_value_pointer_offset, as it can be
inferred from has_prev
================
Comment at: lldb/examples/synthetic/gnu_libstdcpp.py:18
+ self.has_prev = has_prev
+ self.node_value_pointer_offset = node_value_pointer_offset
logger >> "Providing synthetic children for a list named " + \
----------------
remove
================
Comment at: lldb/examples/synthetic/gnu_libstdcpp.py:72
self.count = self.num_children_impl()
- return self.count
+ return self.count
----------------
remove this trailing space
================
Comment at: lldb/examples/synthetic/gnu_libstdcpp.py:85-91
+ prev_val = self.prev.GetValueAsUnsigned(0)
+ if prev_val == 0:
+ return 0
+ if next_val == self.node_address:
+ return 0
+ if next_val == prev_val:
+ return 1
----------------
nice
================
Comment at: lldb/examples/synthetic/gnu_libstdcpp.py:92
+ return 1
+ size = self.node_value_pointer_offset
current = self.next
----------------
let's better initialize this with a simpler value
================
Comment at: lldb/examples/synthetic/gnu_libstdcpp.py:98-99
current = current.GetChildMemberWithName('_M_next')
- return (size - 1)
+ if self.has_prev:
+ return (size-1)
+ return size
----------------
remove this or do any necessary corrections here
================
Comment at: lldb/examples/synthetic/gnu_libstdcpp.py:125-127
return current.CreateChildAtOffset(
'[' + str(index) + ']',
+ self.node_value_pointer_offset *
current.GetType().GetByteSize(),
----------------
================
Comment at: lldb/examples/synthetic/gnu_libstdcpp.py:159-162
+ # This address is used to identify if a node traversal has reached its end
+ # and is mandatory to be overriden in each AbstractListSynthProvider child
class
+ def get_end_of_list_address(self):
+ raise NotImplementedError
----------------
good
================
Comment at: lldb/examples/synthetic/gnu_libstdcpp.py:169-172
+ # Lists have 1 or more pointers followed by the value.
+ # We should specify the number of the pointers for the correct
+ # size determination
+ node_value_pointer_offset = 1
----------------
remove
================
Comment at: lldb/examples/synthetic/gnu_libstdcpp.py:175-182
+ def num_children(self):
+ return super().num_children()
+
+ def get_child_at_index(self, index):
+ return super().get_child_at_index(index)
+
+ def extract_type(self):
----------------
you can delete these
================
Comment at: lldb/examples/synthetic/gnu_libstdcpp.py:184-188
+ def update(self):
+ super().update()
+ self.node = self.impl.GetChildMemberWithName('_M_head')
+ self.next = self.node.GetChildMemberWithName('_M_next')
+ return False
----------------
instead of this, declare a method in the parent class
def updateNodes(self)
... # don't return anything
that is invoked by 'update'.
both children must implement this method. That way you don't need to invoke
super. Also you should write a comment that the 'udpateNodes' method must give
value to self.node and self.next, and optionally to self.prev
================
Comment at: lldb/examples/synthetic/gnu_libstdcpp.py:199-202
+ # Lists have 1 or more pointers followed by the value.
+ # We should specify the number of the pointers for the correct
+ # size determination
+ node_value_pointer_offset = 2
----------------
remove
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D113362/new/
https://reviews.llvm.org/D113362
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
