wallace requested changes to this revision.
wallace added a comment.
This revision now requires changes to proceed.
When you do your tests, don't care too much about std::string yet, as it has
its own issues, but instead create a non-standard size type like
struct Foo {
int a;
int b;
int c;
};
...
std::unordered_map<int, Foo> u;
================
Comment at: lldb/examples/synthetic/gnu_libstdcpp.py:16-24
+ logger = lldb.formatters.Logger.Logger()
+ list_type = self.valobj.GetType().GetUnqualifiedType()
+ if list_type.IsReferenceType():
+ list_type = list_type.GetDereferencedType()
+ if list_type.GetNumberOfTemplateArguments() > 0:
+ data_type = list_type.GetTemplateArgumentType(0)
+ else:
----------------
you can simplify this
================
Comment at: lldb/examples/synthetic/gnu_libstdcpp.py:18
+ list_type = self.valobj.GetType().GetUnqualifiedType()
+ if list_type.IsReferenceType():
+ list_type = list_type.GetDereferencedType()
----------------
you are already passing stl_deref_flags when you declare this formatter in C++,
so you shouldn't get a Reference type here
================
Comment at: lldb/examples/synthetic/gnu_libstdcpp.py:21
+ if list_type.GetNumberOfTemplateArguments() > 0:
+ data_type = list_type.GetTemplateArgumentType(0)
+ else:
----------------
GetTemplateArgumentType(0) will give you the key, and
GetTemplateArgumentType(1) should give you the value type. The idea is that in
this place you get both key_type and value_type, so that you don't need to
figure them out later
================
Comment at: lldb/examples/synthetic/gnu_libstdcpp.py:44
+ def get_child_index(self, name):
+ logger = lldb.formatters.Logger.Logger()
+ try:
----------------
remove if not used
================
Comment at: lldb/examples/synthetic/gnu_libstdcpp.py:55
+ current = self.next
+ while offset > 0:
+ current = current.GetChildMemberWithName('_M_nxt')
----------------
this looks weird. you are already doing a while loop in get_child_at_index that
is taking you to the node you want, but you do again another loop. You have to
use `current` instead. My suggestion is to remove this function and make
everything happen in get_child_at_index with one single loop
================
Comment at: lldb/examples/synthetic/gnu_libstdcpp.py:71
+ try:
+ offset = self.count - index - 1
+ current = self.next
----------------
why do you do this? that means that you are traversing backwards. Just move an
'index' number of times. The unordered map is unordered by definition
================
Comment at: lldb/examples/synthetic/gnu_libstdcpp.py:77
+ key = self.get_child_key(index)
+ return current.CreateChildAtOffset('[' + str(key.GetValue()) +
']', self.next.GetType().GetByteSize() + self.data_size, self.data_type)
+
----------------
danilashtefan wrote:
> I assume that the cause of the first issue (different key value type) is
> located here. self.next.GetType().GetByteSize() + self.data_size does not
> work in this case. Key is read, however value - not. It means that we cannot
> always add self.data_size.
>
> I tried to manually figure out on the unordered_map<char, int> case what can
> I add instead of self.data_size (which is 1 in this case), but got no success
do what i mentioned above regarding fetching the key and value types. I imagine
that the internal allocator is using a pair<key, value>, which means that the
memory layout will be:
key
value [with an offset of key size]
================
Comment at: lldb/examples/synthetic/gnu_libstdcpp.py:77
+ key = self.get_child_key(index)
+ return current.CreateChildAtOffset('[' + str(key.GetValue()) +
']', self.next.GetType().GetByteSize() + self.data_size, self.data_type)
+
----------------
wallace wrote:
> danilashtefan wrote:
> > I assume that the cause of the first issue (different key value type) is
> > located here. self.next.GetType().GetByteSize() + self.data_size does not
> > work in this case. Key is read, however value - not. It means that we
> > cannot always add self.data_size.
> >
> > I tried to manually figure out on the unordered_map<char, int> case what
> > can I add instead of self.data_size (which is 1 in this case), but got no
> > success
> do what i mentioned above regarding fetching the key and value types. I
> imagine that the internal allocator is using a pair<key, value>, which means
> that the memory layout will be:
> key
> value [with an offset of key size]
here you are returning a child of type self.data_type, which is the key type.
And we don't want that. We want to do the same thing that the map formatter
does for getting the pair<key, value> and returning it:
https://github.com/llvm-mirror/lldb/blob/master/examples/synthetic/gnu_libstdcpp.py#L366
Notice how data type in that case is gotten here
https://github.com/llvm-mirror/lldb/blob/master/examples/synthetic/gnu_libstdcpp.py#L367
and is actually the pair. And then they pair is returned here
https://github.com/llvm-mirror/lldb/blob/master/examples/synthetic/gnu_libstdcpp.py#L425
that means that you don't need to individually find the key and the value, you
only need to find the pair and return the pair
Btw, if raw print an unordered_map, I get this:
(std::unordered_map<int, double, std::hash<int>, std::equal_to<int>,
std::allocator<std::pair<const int, double> > >)
that means that the fifth template argument contains the pair. I've confirmed
that also libcxx works the same way
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D113760/new/
https://reviews.llvm.org/D113760
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
