Page MenuHomePhabricator

[formatters] Add a deque formatter for libstdcpp and fix the libcxx one
AcceptedPublic

Authored by wallace on Tue, Nov 16, 9:11 AM.

Details

Reviewers
danilashtefan
Summary

This adds the formatter for libstdcpp's deque as a python
implementation. It adds comprehensive tests for the two different
storage strategies deque uses. Besides that, this fixes a couple of bugs
in the libcxx implementation. Finally, both implementation run against
the same tests.

This is a minor improvement on top of Danil Stefaniuc's formatter.

Diff Detail

Event Timeline

danilashtefan requested review of this revision.Tue, Nov 16, 9:11 AM
danilashtefan created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptTue, Nov 16, 9:11 AM
danilashtefan added inline comments.Tue, Nov 16, 9:24 AM
lldb/examples/synthetic/gnu_libstdcpp.py
180

This method does not currently work. I simply tried to calculate the size from start and finish pointers. When it did not, I investigated further and this was my founding:

This is when we get the iterator value this comes out:

`_Deque_iterator(const _Deque_iterator& __x) noexcept

169 	       : _M_cur(__x._M_cur), _M_first(__x._M_first),
170 		 _M_last(__x._M_last), _M_node(__x._M_node) { }`

And when it++ is done this is what happens behind:

operator++()

189       {
190    ++_M_cur;
191    if (_M_cur == _M_last)
192      {
193        _M_set_node(_M_node + 1);
194        _M_cur = _M_first;
195      }
196    return *this;
197        }

When I mocked the num_children to return 4, formatter works for any structure with size <= 4. here is the example:

std::deque<int> d;
 d.push_front(7);
 d.push_front(5);   
 d.push_front(13);
 d.push_front(25);

(std::deque<int, std::allocator<int> >) $0 = size=4 {

[0] = 25
[1] = 13
[2] = 5
[3] = 7

}

wallace requested changes to this revision.Wed, Nov 17, 8:26 AM

discussed offline how to traverse each individual subarray. This might make it impossible to reuse it for queue and stack

lldb/examples/synthetic/gnu_libstdcpp.py
133

remove

142

remove

151

don't forget to always return False

180

discussed offline how to handle this

This revision now requires changes to proceed.Wed, Nov 17, 8:26 AM
danilashtefan updated this revision to Diff 388745.EditedSun, Nov 21, 3:05 AM

Hi Walter :)

I added the size determination here. The commented out set_node node method should switch the nodes, but, unfortunately, after numerous of tries, I did not succeed. The prints are left only for a draft PR and I will delete them in the final diff update, thank you!

Unified tests added

As we discussed offline, I add the print tests, since SBValue.GetValue() doesn't work for complex structures

labath added a subscriber: labath.Thu, Nov 25, 2:35 AM

As we discussed offline, I add the print tests, since SBValue.GetValue() doesn't work for complex structures

There are different ways you can do that depending on what you want to test precisely. You can (programatically, using loops?) construct nested ValueCheck structures and pass that to the expect_var_path .

Or you can, issue a bunch of smaller expect_var_path commands targetting individual subobjects, again using loops (expect_var_path("myobj[%d]"%i, ...).

Hi @labath, Thank you so much for your comment and correction. The initial version of the test was the following, with array of ValueCheck objects:

`def check(self, var_name, size):

var = self.findVariable(var_name)
self.assertEqual(var.GetNumChildren(), size)
children = []
for i in range(100):
    children.insert(0,ValueCheck(value={i, i + 1, i + 2}))
    children.append(ValueCheck(value={-i, -(i + 1), -(i + 2)}))
self.expect_var_path(var_name, type=self.getVariableType(var_name), children=children)`

However, the following error pops up in the expected_var_path: AssertionError: {99, 100, 101} != None : Checking child with index 0:. The reason is that child.GetValue()that is invoked inside gives None in case it is a complex structure. This is why me and @wallace decided to go with a printing test.

If I did not get you right, I would be glad if you could describe your solution more, so I can implement it. Many thanks!

You need to construct issue appropriately nested ValueCheck objects. Something like this:

children.insert(0,ValueCheck(children=[ValueCheck(value=i), ValueCheck(value=i+1), ValueCheck(i + 2)])

If that doesn't work then that's probably a bug in the ValueCheck matching implementation, as it was definitely written with that mind.

wallace commandeered this revision.Thu, Dec 2, 5:52 PM
wallace edited reviewers, added: danilashtefan; removed: wallace.
wallace updated this revision to Diff 391513.Thu, Dec 2, 5:55 PM

Fix the few issues in Danil's implementation

wallace retitled this revision from Draft PR for the deque, stack, queue lldb data formatters to [formatters] Add a deque formatter for libstdcpp and fix the libcxx one.Thu, Dec 2, 5:56 PM
wallace edited the summary of this revision. (Show Details)
danilashtefan accepted this revision.Fri, Dec 3, 2:26 AM
This revision is now accepted and ready to land.Fri, Dec 3, 2:26 AM