[formatters] Add a libstdcpp formatter for forward_list and refactor list formatter

[danilashtefan]

Summary

This diff adds a data formatter for libstdcpp's forward_list. Besides, it refactors the existing code by extracting the common functionality between libstdcpp forward_list and list formatters into the AbstractListSynthProvider class.

[danilashtefan]

Hi Walter:) Please, could you advise what can I change/do more? As a next step, I will unify the tests across the libcxx and libstdcpp formatters.

[wallace]

very good start!

lldb/examples/synthetic/gnu_libstdcpp.py

5–6

remove this

21–151

remove all these _abstract suffixes. They make reading harder

27

here you should call the method get_end_of_list_address that i mention below

41–42

don't remove this command. It's useful to know what's going on

95

what is this? try to come up with a better name with documentation to make this easier to understand

161

don't pass the is_valid_break_condition variable everywhere. It makes code more confusing. Instead, you should create a method "get_end_of_list_address" that should be overridden by each child implementation. Then you add a comment that this address is used to identify if a node traversal has reached its end

161

don't pass 'type' here, instead, in the constructor of the parent in line 158 pass a boolean flag 'has_prev'. That should be enough for the parent class to know if there's a prev pointer or not

161

similarly, pass incoming_size to the constructor, and a better name is "node_value_pointer_offset", and you add a comment explaining that lists have 1 or more pointers followed by the value

[danilashtefan]

All of the requested changes are done. Please, let me know if any other corrections are needed

[wallace]

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

lldb/examples/synthetic/gnu_libstdcpp.py

15–17

Add these comments and remove the node_value_pointer_offset, as it can be inferred from has_prev

19

remove

68–69

remove this trailing space

82–88

nice

89

let's better initialize this with a simpler value

95–96

remove this or do any necessary corrections here

120–122

154–157

good

164–167

remove

170–177

you can delete these

179–183

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

194–197

remove

[danilashtefan]

All the requested changes are done :)

[danilashtefan]

lldb/examples/synthetic/gnu_libstdcpp.py

89

Initial size does also depend on the has_prev value, so I decided to initialize it is this simple way

95

Size to return also depends on the has_prev. I have simplified the previously introduced if case

[danilashtefan]

Right size is returned and tests are unified! :)

[wallace]

thanks!

lldb/examples/synthetic/gnu_libstdcpp.py

 from __future__ import division
 import re
 import lldb.formatters.Logger
 
+FORWARD_LIST = "FORWARD_LIST"
+LIST = "LIST"
+
 # C++ STL formatters for LLDB
 # These formatters are based upon the version of the GNU libstdc++
 # as it ships with Mac OS X 10.6.8 thru 10.8.0
 # You are encouraged to look at the STL implementation for your platform
 # before relying on these formatters to do the right thing for your setup
-
-
-class StdListSynthProvider:
-
+class AbstractListSynthProvider:
     def __init__(self, valobj, dict):
         logger = lldb.formatters.Logger.Logger()
         self.valobj = valobj
         self.count = None
         logger >> "Providing synthetic children for a list named " + \
             str(valobj.GetName())
 
-    def next_node(self, node):
+    def next_node_abstract(self, node):
         logger = lldb.formatters.Logger.Logger()
         return node.GetChildMemberWithName('_M_next')
 
-    def is_valid(self, node):
+    def is_valid_abstract(self, node, break_condition):
         logger = lldb.formatters.Logger.Logger()
-        valid = self.value(self.next_node(node)) != self.node_address
+        valid = self.value_abstract(self.next_node_abstract(node)) != break_condition
         if valid:
             logger >> "%s is valid" % str(self.valobj.GetName())
         else:
             logger >> "synthetic value is not valid"
         return valid
     
-    def value(self, node):
+    def value_abstract(self, node):
         logger = lldb.formatters.Logger.Logger()
         value = node.GetValueAsUnsigned()
         logger >> "synthetic value for {}: {}".format(
             str(self.valobj.GetName()), value)
         return value
 
-    # Floyd's cycle-finding algorithm
-    # try to detect if this list has a loop
-    def has_loop(self):
+    def has_loop_abstract(self, is_valid_break_condition):
         global _list_uses_loop_detector
         logger = lldb.formatters.Logger.Logger()
         if not _list_uses_loop_detector:
             logger >> "Asked not to use loop detection"
             return False
         slow = self.next
         fast1 = self.next
         fast2 = self.next
-        while self.is_valid(slow):
-            slow_value = self.value(slow)
-            fast1 = self.next_node(fast2)
-            fast2 = self.next_node(fast1)
-            if self.value(fast1) == slow_value or self.value(
+        while self.is_valid_abstract(slow, is_valid_break_condition):
+            slow_value = self.value_abstract(slow)
+            fast1 = self.next_node_abstract(fast2)
+            fast2 = self.next_node_abstract(fast1)
+            if self.value_abstract(fast1) == slow_value or self.value_abstract(
                     fast2) == slow_value:
                 return True
-            slow = self.next_node(slow)
+            slow = self.next_node_abstract(slow)
         return False
 
-    def num_children(self):
+    def num_children_abstract(self, incoming_size, size_correction, is_valid_break_condition, type):
         logger = lldb.formatters.Logger.Logger()
         if self.count is None:
             # libstdc++ 6.0.21 added dedicated count field.
             count_child = self.node.GetChildMemberWithName('_M_data')
             if count_child and count_child.IsValid():
                 self.count = count_child.GetValueAsUnsigned(0)
             if self.count is None:
-                self.count = self.num_children_impl()
-        return self.count
+                self.count = self.num_children_impl_abstract(incoming_size,size_correction, is_valid_break_condition, type)
+        return self.count 
 
-    def num_children_impl(self):
+    def num_children_impl_abstract(self, incoming_size, size_correction, is_valid_break_condition, type):
         logger = lldb.formatters.Logger.Logger()
         try:
-            next_val = self.next.GetValueAsUnsigned(0)
-            prev_val = self.prev.GetValueAsUnsigned(0)
             # After a std::list has been initialized, both next and prev will
             # be non-NULL
-            if next_val == 0 or prev_val == 0:
+            next_val = self.next.GetValueAsUnsigned(0)
+            if next_val == 0: 
+                return 0
+            if self.has_loop_abstract(is_valid_break_condition):
+                return 0
+            if type == LIST:
+                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
-            if self.has_loop():
-                return 0
-            size = 2
+                    return 1               
+            size = incoming_size
             current = self.next
             while current.GetChildMemberWithName(
-                    '_M_next').GetValueAsUnsigned(0) != self.node_address:
+                    '_M_next').GetValueAsUnsigned(0) != is_valid_break_condition:
                 size = size + 1
                 current = current.GetChildMemberWithName('_M_next')
-            return (size - 1)
+            return (size - size_correction)
         except:
-            return 0
+            logger >> "Error determining the size"
+            return 0 
 
-    def get_child_index(self, name):
+    def get_child_index_abstract(self, name):
         logger = lldb.formatters.Logger.Logger()
         try:
             return int(name.lstrip('[').rstrip(']'))
         except:
             return -1
       
-    def get_child_at_index(self, index):
+    def get_child_at_index_abstract(self, index, multiplier):
         logger = lldb.formatters.Logger.Logger()
         logger >> "Fetching child " + str(index)
         if index < 0:
             return None
         if index >= self.num_children():
             return None
         try:
             offset = index
             current = self.next
             while offset > 0:
                 current = current.GetChildMemberWithName('_M_next')
                 offset = offset - 1
             return current.CreateChildAtOffset(
                 '[' + str(index) + ']',
-                2 * current.GetType().GetByteSize(),
+               multiplier * current.GetType().GetByteSize(),
                 self.data_type)
         except:
             return None
 
-    def extract_type(self):
+    def extract_type_abstract(self):
         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:
             data_type = None
         return data_type
 
-    def update(self):
+    def update_abstract(self):
         logger = lldb.formatters.Logger.Logger()
         # preemptively setting this to None - we might end up changing our mind
         # later
         self.count = None
         try:
-            impl = self.valobj.GetChildMemberWithName('_M_impl')
-            self.node = impl.GetChildMemberWithName('_M_node')
-            self.node_address = self.valobj.AddressOf().GetValueAsUnsigned(0)
-            self.next = self.node.GetChildMemberWithName('_M_next')
-            self.prev = self.node.GetChildMemberWithName('_M_prev')
-            self.data_type = self.extract_type()
+            self.impl = self.valobj.GetChildMemberWithName('_M_impl')
+            self.data_type = self.extract_type_abstract()
             self.data_size = self.data_type.GetByteSize()
         except:
             pass
         return False
 
-    def has_children(self):
+    def has_children_abstract(self):
         return True
 
 
+class StdForwardListSynthProvider(AbstractListSynthProvider):
+
+    def __init__(self, valobj, dict):
+        super().__init__(valobj, dict)
+    
+    def num_children(self):
+        return super().num_children_abstract(incoming_size=1, size_correction=0, is_valid_break_condition=0, type=FORWARD_LIST)
+
+
+    def get_child_at_index(self, index):
+        return super().get_child_at_index_abstract(index=index,multiplier=1)
+
+    def extract_type(self):
+        return super().extract_type_abstract()
+
+    def update(self):
+        super().update_abstract()
+        self.node = self.impl.GetChildMemberWithName('_M_head')
+        self.next = self.node.GetChildMemberWithName('_M_next')
+        return False
+
+
+class StdListSynthProvider(AbstractListSynthProvider):
+
+    def __init__(self, valobj, dict):
+        super().__init__(valobj, dict)
+
+    def num_children(self):
+        return super().num_children_abstract(incoming_size=2, size_correction=1, is_valid_break_condition=self.node_address, type=LIST)
+
+    def get_child_at_index(self, index):
+        return super().get_child_at_index_abstract(index=index,multiplier=2)
+
+    def update(self):
+        self.update_abstract()
+        self.node_address = self.valobj.AddressOf().GetValueAsUnsigned(0)
+        self.node = self.impl.GetChildMemberWithName('_M_node')
+        self.prev = self.node.GetChildMemberWithName('_M_prev')
+        self.next = self.node.GetChildMemberWithName('_M_next')
+        return False
+
+
 class StdVectorSynthProvider:
 
     class StdVectorImplementation(object):
 
         def __init__(self, valobj):
             self.valobj = valobj
             self.count = None
 
         def num_children(self):

lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp

cpp_category_sp->GetRegexTypeSyntheticsContainer()->Add(
       SyntheticChildrenSP(new ScriptedSyntheticChildren(
           stl_deref_flags,
           "lldb.formatters.cpp.gnu_libstdcpp.StdMapLikeSynthProvider")));
   cpp_category_sp->GetRegexTypeSyntheticsContainer()->Add(
       RegularExpression("^std::(__cxx11::)?list<.+>(( )?&)?$"),
       SyntheticChildrenSP(new ScriptedSyntheticChildren(
           stl_synth_flags,
           "lldb.formatters.cpp.gnu_libstdcpp.StdListSynthProvider")));
+  cpp_category_sp->GetRegexTypeSyntheticsContainer()->Add(
+      RegularExpression("^std::(__cxx11::)?forward_list<.+>(( )?&)?$"),
+      SyntheticChildrenSP(new ScriptedSyntheticChildren(
+          stl_synth_flags,
+          "lldb.formatters.cpp.gnu_libstdcpp.StdForwardListSynthProvider")));
   stl_summary_flags.SetDontShowChildren(false);
   stl_summary_flags.SetSkipPointers(false);
   cpp_category_sp->GetRegexTypeSummariesContainer()->Add(
       RegularExpression("^std::bitset<.+>(( )?&)?$"),
       TypeSummaryImplSP(
           new StringSummaryFormat(stl_summary_flags, "size=${svar%#}")));
   cpp_category_sp->GetRegexTypeSummariesContainer()->Add(
       RegularExpression("^std::vector<.+>(( )?&)?$"),
static void LoadLibStdcppFormatters(lldb::TypeCategoryImplSP cpp_category_sp) {
   cpp_category_sp->GetRegexTypeSummariesContainer()->Add(
       RegularExpression("^std::multiset<.+> >(( )?&)?$"),
       TypeSummaryImplSP(
           new StringSummaryFormat(stl_summary_flags, "size=${svar%#}")));
   cpp_category_sp->GetRegexTypeSummariesContainer()->Add(
       RegularExpression("^std::(__cxx11::)?list<.+>(( )?&)?$"),
       TypeSummaryImplSP(
           new StringSummaryFormat(stl_summary_flags, "size=${svar%#}")));
+  cpp_category_sp->GetRegexTypeSummariesContainer()->Add(
+      RegularExpression("^std::(__cxx11::)?forward_list<.+>(( )?&)?$"),
+      TypeSummaryImplSP(
+          new StringSummaryFormat(stl_summary_flags, "size=${svar%#}")));
 
   AddCXXSynthetic(
       cpp_category_sp,
       lldb_private::formatters::LibStdcppVectorIteratorSyntheticFrontEndCreator,
       "std::vector iterator synthetic children",
       ConstString("^__gnu_cxx::__normal_iterator<.+>$"), stl_synth_flags, true);
 
   AddCXXSynthetic(

lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/Makefile

+CXX_SOURCES := main.cpp
+
+USE_LIBSTDCPP := 1
+include Makefile.rules

lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/TestDataFormatterStdForwardList.py

+"""
+Test lldb data formatter subsystem.
+"""
+
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestDataFormatterLibcxxForwardList(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    def setUp(self):
+        TestBase.setUp(self)
+        self.line = line_number('main.cpp', '// break here')
+        self.namespace = 'std'
+
+    @add_test_categories(["libstdcxx"])
+    def test(self):
+        """Test that std::forward_list is displayed correctly"""
+        self.build()
+        lldbutil.run_to_source_breakpoint(self, '// break here',
+                lldb.SBFileSpec("main.cpp", False))
+
+        forward_list = self.namespace + '::forward_list'
+        self.expect("frame variable empty",
+                    substrs=[forward_list,
+                             'size=0',
+                             '{}'])
+
+        self.expect("frame variable one_elt",
+                    substrs=[forward_list,
+                             'size=1',
+                             '{',
+                             '[0] = 47',
+                             '}'])
+
+        self.expect("frame variable five_elts",
+                    substrs=[forward_list,
+                             'size=5',
+                             '{',
+                             '[0] = 1',
+                             '[1] = 22',
+                             '[2] = 333',
+                             '[3] = 4444',
+                             '[4] = 55555',
+                             '}'])

lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/forward_list/main.cpp

+#include <forward_list>
+
+int main() {
+  std::forward_list<int> empty{}, one_elt{47},
+      five_elts{1, 22, 333, 4444, 55555};
+  return 0; // break here
+}