[formatters] Add a libstdcpp formatter for set and unify tests across stdlibs

[danilashtefan]

Summary

This diff adds a data formatter for libstdcpp's set. Besides, it unifies the tests for set for libcxx and libstdcpp for maintainability.

[danilashtefan]

Set synthetic children

[danilashtefan]

Set synthetic child

[danilashtefan]

Deleted libcxx tests

[wallace]

I think you messed up when updating the diffs. I don't see the bitset diff anymore.

[wallace]

just some minor things left

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

930–931

to mimic the libcxx case that you can see in the line 710 of https://lldb.llvm.org/cpp_reference/CPlusPlusLanguage_8cpp_source.html, you can do stl_summary_flags.SetSkipPointers(false) in line 932 of this file. Then both standard libraries will be handled the same way. Make sure to execute the relevant tests for vector, map, set and list for libstdcpp.
This also means that you don't need the new variable stl_set_summary_flags

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

2–3

remove this line

lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/TestDataFormatterGenericSet.py

32–39

can you add additional assertions like the one you did in https://reviews.llvm.org/D112180 that uses ValueCheck?

[wallace]

Besides, given that you are using the same python implementation for map and set, better rename the class to StdSeOrMapSynthProvider and add a comment mentioning that it works both for set and maps because they have the same underlying structure

[danilashtefan]

Thank you for the comments, I have done everything specified!

[danilashtefan]

lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/TestDataFormatterGenericSet.py

32–39

I have made check method more generic and less verbose. Before it was specifically checking the "ii" variable, that was a set<int> of size 7. With value check approach we can now test set<string> and others.

I guess, that I can delete all of the

self.expect()

and simply call check method with the expected size. Please, let me know if I should do it or leave it as it is. Thanks

[danilashtefan]

Small formatting

[wallace]

pretty good! Just some cosmetic changes needed and that's it

lldb/examples/synthetic/gnu_libstdcpp.py

317–321

Make a comment above like

'''
Set and Map have the same underlying data structure, therefore we can use exactly the same implementation for the formatter.
'''

327–329

As this is not just for maps, you can do something like:

self.kind = "set" if "set" in valobj.GetTypeName() else "map"
logger >> "Providing synthetic children for a " self.kind + " named " + \
          str(valobj.GetName())

356

lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/TestDataFormatterGenericSet.py

153–154

remove these lines

[wallace]

lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/TestDataFormatterGenericSet.py

32–39

yes, the code is good as you did it

[danilashtefan]

Refactoring

[wallace]

Just fix that last thing and I'll land your patch :) Thanks

lldb/examples/synthetic/gnu_libstdcpp.py

317–321

actual python documentation uses ''' for comments, not #

[danilashtefan]

Last thing corrected :)

lldb/examples/synthetic/gnu_libstdcpp.py

def get_child_at_index(self, index):
         return self.impl.get_child_at_index(index)
 
     def update(self):
         return self.impl.update()
 
     def has_children(self):
         return True
 
-
-class StdMapSynthProvider:
+    """
+    Set and Map have the same underlying data structure,
+    therefore we can use exactly the same implementation for the formatter.
+    """
+class StdSetOrMapSynthProvider:
 
     def __init__(self, valobj, dict):
         logger = lldb.formatters.Logger.Logger()
         self.valobj = valobj
         self.count = None
-        logger >> "Providing synthetic children for a map named " + \
+        self.kind = "set" if "set" in valobj.GetTypeName() else "map"
+        logger >> "Providing synthetic children for a " + self.kind + " named " + \
             str(valobj.GetName())
 
     # we need this function as a temporary workaround for rdar://problem/10801549
     # which prevents us from extracting the std::pair<K,V> SBType out of the template
-    # arguments for _Rep_Type _M_t in the map itself - because we have to make up the
+    # arguments for _Rep_Type _M_t in the object itself - because we have to make up the
     # typename and then find it, we may hit the situation were std::string has multiple
     # names but only one is actually referenced in the debug information. hence, we need
     # to replace the longer versions of std::string with the shorter one in order to be able
     # to find the type name
     def fixup_class_name(self, class_name):
         logger = lldb.formatters.Logger.Logger()
         if class_name == 'std::basic_string<char, std::char_traits<char>, std::allocator<char> >':
             return 'std::basic_string<char>', True
         if class_name == 'basic_string<char, std::char_traits<char>, std::allocator<char> >':
             return 'std::basic_string<char>', True
         if class_name == 'std::basic_string<char, std::char_traits<char>, std::allocator<char> >':
             return 'std::basic_string<char>', True
         if class_name == 'basic_string<char, std::char_traits<char>, std::allocator<char> >':
             return 'std::basic_string<char>', True
         return class_name, False
 
     def update(self):
         logger = lldb.formatters.Logger.Logger()
         # preemptively setting this to None - we might end up changing our mind
         # later
         self.count = None
         try:
-            # we will set this to True if we find out that discovering a node in the map takes more steps than the overall size of the RB tree
+            # we will set this to True if we find out that discovering a node in the object takes more steps than the overall size of the RB tree
             # if this gets set to True, then we will merrily return None for
             # any child from that moment on
             self.garbage = False
             self.Mt = self.valobj.GetChildMemberWithName('_M_t')
             self.Mimpl = self.Mt.GetChildMemberWithName('_M_impl')
             self.Mheader = self.Mimpl.GetChildMemberWithName('_M_header')
 
             map_type = self.valobj.GetType()

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

cpp_category_sp->GetRegexTypeSyntheticsContainer()->Add(
       RegularExpression("^std::vector<.+>(( )?&)?$"),
       SyntheticChildrenSP(new ScriptedSyntheticChildren(
           stl_synth_flags,
           "lldb.formatters.cpp.gnu_libstdcpp.StdVectorSynthProvider")));
   cpp_category_sp->GetRegexTypeSyntheticsContainer()->Add(
       RegularExpression("^std::map<.+> >(( )?&)?$"),
       SyntheticChildrenSP(new ScriptedSyntheticChildren(
           stl_synth_flags,
-          "lldb.formatters.cpp.gnu_libstdcpp.StdMapSynthProvider")));
+          "lldb.formatters.cpp.gnu_libstdcpp.StdSetOrMapSynthProvider")));
+  cpp_category_sp->GetRegexTypeSyntheticsContainer()->Add(
+      RegularExpression("^std::set<.+> >(( )?&)?$"),
+      SyntheticChildrenSP(new ScriptedSyntheticChildren(
+          stl_deref_flags,
+          "lldb.formatters.cpp.gnu_libstdcpp.StdSetOrMapSynthProvider")));
   cpp_category_sp->GetRegexTypeSyntheticsContainer()->Add(
       RegularExpression("^std::(__cxx11::)?list<.+>(( )?&)?$"),
       SyntheticChildrenSP(new ScriptedSyntheticChildren(
           stl_synth_flags,
           "lldb.formatters.cpp.gnu_libstdcpp.StdListSynthProvider")));
   stl_summary_flags.SetDontShowChildren(false);
-  stl_summary_flags.SetSkipPointers(true);
+  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<.+>(( )?&)?$"),
       TypeSummaryImplSP(
           new StringSummaryFormat(stl_summary_flags, "size=${svar%#}")));
   cpp_category_sp->GetRegexTypeSummariesContainer()->Add(
       RegularExpression("^std::map<.+> >(( )?&)?$"),
       TypeSummaryImplSP(
           new StringSummaryFormat(stl_summary_flags, "size=${svar%#}")));
   cpp_category_sp->GetRegexTypeSummariesContainer()->Add(
+      RegularExpression("^std::set<.+> >(( )?&)?$"),
+      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%#}")));
 
   AddCXXSynthetic(
       cpp_category_sp,
       lldb_private::formatters::LibStdcppVectorIteratorSyntheticFrontEndCreator,
       "std::vector iterator synthetic children",

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

 CXX_SOURCES := main.cpp
 
-USE_LIBCPP := 1
-
-CXXFLAGS_EXTRAS := -O0
 include Makefile.rules

lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/set/TestDataFormatterGenericSet.py

 """
 Test lldb data formatter subsystem.
 """
 
 
 
 import lldb
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
+USE_LIBSTDCPP = "USE_LIBSTDCPP"
+USE_LIBCPP = "USE_LIBCPP"
 
 class LibcxxSetDataFormatterTestCase(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
 
     def setUp(self):
         TestBase.setUp(self)
         self.namespace = 'std'
 
-    def getVariableType(self, name):
+    def findVariable(self, name):
         var = self.frame().FindVariable(name)
         self.assertTrue(var.IsValid())
+        return var
+
+    def getVariableType(self, name):
+        var = self.findVariable(name)
         return var.GetType().GetDisplayTypeName()
 
-    def check_ii(self, var_name):
-        """ This checks the value of the bitset stored in ii at the call to by_ref_and_ptr.
-            We use this to make sure we get the same values for ii when we look at the object
-            directly, and when we look at a reference to the object. """
-        self.expect(
-            "frame variable " + var_name,
-            substrs=["size=7",
-                     "[2] = 2",
-                     "[3] = 3",
-                     "[6] = 6"])
-        self.expect("frame variable " + var_name + "[2]", substrs=[" = 2"])
-        self.expect(
-            "p " + var_name,
-            substrs=[
-                "size=7",
-                "[2] = 2",
-                "[3] = 3",
-                "[6] = 6"])
+    def check(self, var_name, size):
+        var = self.findVariable(var_name)
+        self.assertEqual(var.GetNumChildren(), size)
+        children = []
+        for i in range(size):
+            child = var.GetChildAtIndex(i)
+            children.append(ValueCheck(value=child.GetValue()))
+        self.expect_var_path(var_name, type=self.getVariableType(var_name), children=children)
 
-    @add_test_categories(["libc++"])
-    def test_with_run_command(self):
+
+
+    def do_test_with_run_command(self,stdlib_type):
         """Test that that file and class static variables display correctly."""
-        self.build()
+        self.build(dictionary={stdlib_type: "1"})
         (self.target, process, _, bkpt) = lldbutil.run_to_source_breakpoint(
             self, "Set break point at this line.", lldb.SBFileSpec("main.cpp", False))
 
         # This is the function to remove the custom formats in order to have a
         # clean slate for the next test case.
         def cleanup():
             self.runCmd('type format clear', check=False)
             self.runCmd('type summary clear', check=False)
def do_test_with_run_command(self,stdlib_type):
             substrs=["size=6",
                      "[0] = 0",
                      "[1] = 1",
                      "[2] = 2",
                      "[3] = 3",
                      "[4] = 4",
                      "[5] = 5"])
         lldbutil.continue_to_breakpoint(process, bkpt)
-        self.check_ii("ii")
+        self.check("ii",7)
 
         lldbutil.continue_to_breakpoint(process, bkpt)
         self.expect("frame variable ii", substrs=["size=0", "{}"])
         lldbutil.continue_to_breakpoint(process, bkpt)
         self.expect("frame variable ii", substrs=["size=0", "{}"])
 
         ss_type = self.getVariableType("ss")
         self.assertTrue(ii_type.startswith(self.namespace + "::set"),
def do_test_with_run_command(self,stdlib_type):
         self.expect("frame variable ss[2]", substrs=[' = "b"'])
         lldbutil.continue_to_breakpoint(process, bkpt)
         self.expect(
             "frame variable ss",
             substrs=["size=3",
                      '[0] = "a"',
                      '[1] = "a very long string is right here"',
                      '[2] = "c"'])
+        self.check("ss",3)
+
+    @add_test_categories(["libstdcxx"])          
+    def test_with_run_command_libstdcpp(self):
+        self.do_test_with_run_command(USE_LIBSTDCPP)
 
     @add_test_categories(["libc++"])
-    def test_ref_and_ptr(self):
+    def test_with_run_command_libcpp(self):
+        self.do_test_with_run_command(USE_LIBCPP)
+
+
+    def do_test_ref_and_ptr(self,stdlib_type):
         """Test that the data formatters work on ref and ptr."""
         self.build()
         (self.target, process, _, bkpt) = lldbutil.run_to_source_breakpoint(
             self, "Stop here to check by ref and ptr.",
             lldb.SBFileSpec("main.cpp", False))
         # The reference should print just like the value:
-        self.check_ii("ref")
+        self.check("ref", 7)
+        self.check("ptr", 7)
 
         self.expect("frame variable ptr",
                     substrs=["ptr =", "size=7"])
         self.expect("expr ptr",
                     substrs=["size=7"])
 
+    @add_test_categories(["libstdcxx"]) 
+    def test_ref_and_ptr_libstdcpp(self):
+        self.do_test_ref_and_ptr(USE_LIBSTDCPP)
+    
+    @add_test_categories(["libc++"])
+    def test_ref_and_ptr_libcpp(self):
+        self.do_test_ref_and_ptr(USE_LIBCPP)
 No newline at end of file

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

+#include <set>
+#include <string>
+
+int g_the_foo = 0;
+
+int thefoo_rw(int arg = 1) {
+  if (arg < 0)
+    arg = 0;
+  if (!arg)
+    arg = 1;
+  g_the_foo += arg;
+  return g_the_foo;
+}
+
+void by_ref_and_ptr(std::set<int> &ref, std::set<int> *ptr) {
+  // Stop here to check by ref and ptr
+  return;
+}
+
+int main() {
+  std::set<int> ii;
+  thefoo_rw(1); // Set break point at this line.
+
+  ii.insert(0);
+  ii.insert(1);
+  ii.insert(2);
+  ii.insert(3);
+  ii.insert(4);
+  ii.insert(5);
+  thefoo_rw(1); // Set break point at this line.
+
+  ii.insert(6);
+  thefoo_rw(1); // Set break point at this line.
+
+  by_ref_and_ptr(ii, &ii);
+
+  ii.clear();
+  thefoo_rw(1); // Set break point at this line.
+
+  std::set<std::string> ss;
+  thefoo_rw(1); // Set break point at this line.
+
+  ss.insert("a");
+  ss.insert("a very long string is right here");
+  thefoo_rw(1); // Set break point at this line.
+
+  ss.insert("b");
+  ss.insert("c");
+  thefoo_rw(1); // Set break point at this line.
+
+  ss.erase("b");
+  thefoo_rw(1); // Set break point at this line.
+
+  return 0;
+}

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/set/main.cpp

-#include <string>
-#include <set>
-
-int g_the_foo = 0;
-
-int thefoo_rw(int arg = 1)
-{
-  if (arg < 0)
-    arg = 0;
-  if (!arg)
-    arg = 1;
-  g_the_foo += arg;
-  return g_the_foo;
-}
-
-void by_ref_and_ptr(std::set<int> &ref, std::set<int> *ptr)
-{
-    // Stop here to check by ref and ptr
-    return;
-} 
-
-int main()
-{
-    std::set<int> ii;
-    thefoo_rw(1);  // Set break point at this line.
-  
-  ii.insert(0);
-  ii.insert(1);
-  ii.insert(2);
-  ii.insert(3);
-  ii.insert(4);
-  ii.insert(5);
-    thefoo_rw(1);  // Set break point at this line.
-
-  ii.insert(6);
-  thefoo_rw(1);  // Set break point at this line.
-
-        by_ref_and_ptr(ii, &ii);
-
-  ii.clear();
-  thefoo_rw(1);  // Set break point at this line.
-
-  std::set<std::string> ss;
-  thefoo_rw(1);  // Set break point at this line.
-
-  ss.insert("a");
-  ss.insert("a very long string is right here");
-  thefoo_rw(1);  // Set break point at this line.
-
-  ss.insert("b");
-  ss.insert("c");
-  thefoo_rw(1);  // Set break point at this line.
-  
-  ss.erase("b");
-  thefoo_rw(1);  // Set break point at this line.
-
-    return 0;
-}