[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

941–942

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

4

remove this line

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

45

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

45

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

318 ↗

(On Diff #382668)

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.
'''

324–325 ↗

(On Diff #382668)

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())

352 ↗

(On Diff #382668)

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

45

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

318 ↗

(On Diff #382668)

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

[danilashtefan]

Last thing corrected :)

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

static void LoadLibStdcppFormatters(lldb::TypeCategoryImplSP cpp_category_sp) {
   cpp_category_sp->GetTypeSummariesContainer()->Add(
       ConstString("std::__cxx11::basic_string<wchar_t, "
                   "std::char_traits<wchar_t>, std::allocator<wchar_t> >"),
       cxx11_wstring_summary_sp);
 
   SyntheticChildren::Flags stl_synth_flags;
   stl_synth_flags.SetCascades(true).SetSkipPointers(false).SetSkipReferences(
       false);
+  SyntheticChildren::Flags stl_deref_flags = stl_synth_flags;
+  stl_deref_flags.SetFrontEndWantsDereference();
 
   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")));
   cpp_category_sp->GetRegexTypeSyntheticsContainer()->Add(
+      RegularExpression("^std::set<.+> >(( )?&)?$"),
+      SyntheticChildrenSP(new ScriptedSyntheticChildren(
+          stl_synth_flags,
+          "lldb.formatters.cpp.gnu_libstdcpp.StdMapSynthProvider")));
+  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);
   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%#}")));
+  TypeSummaryImpl::Flags stl_set_summary_flags = stl_summary_flags;
+  stl_set_summary_flags.SetSkipPointers(false).SetDontShowValue(false);
+  cpp_category_sp->GetRegexTypeSummariesContainer()->Add(
+      RegularExpression("^std::set<.+> >(( )?&)?$"),
+      TypeSummaryImplSP(
+          new StringSummaryFormat(stl_set_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,

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

+CXX_SOURCES := main.cpp
+
+
+
+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):
+        var = self.frame().FindVariable(name)
+        self.assertTrue(var.IsValid())
+        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 do_test_with_run_command(self,stdlib_type):
+        """Test that that file and class static variables display correctly."""
+        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)
+            self.runCmd('type filter clear', check=False)
+            self.runCmd('type synth clear', check=False)
+            self.runCmd(
+                "settings set target.max-children-count 256",
+                check=False)
+
+        # Execute the cleanup function during test case tear down.
+        self.addTearDownHook(cleanup)
+
+        ii_type = self.getVariableType("ii")
+        self.assertTrue(ii_type.startswith(self.namespace + "::set"),
+                        "Type: " + ii_type)
+
+        self.expect("frame variable ii", substrs=["size=0", "{}"])
+        lldbutil.continue_to_breakpoint(process, bkpt)
+        self.expect(
+            "frame variable ii",
+            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")
+
+        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"),
+                        "Type: " + ss_type)
+
+        self.expect("frame variable ss", substrs=["size=0", "{}"])
+        lldbutil.continue_to_breakpoint(process, bkpt)
+        self.expect(
+            "frame variable ss",
+            substrs=["size=2",
+                     '[0] = "a"',
+                     '[1] = "a very long string is right here"'])
+        lldbutil.continue_to_breakpoint(process, bkpt)
+        self.expect(
+            "frame variable ss",
+            substrs=["size=4",
+                     '[0] = "a"',
+                     '[1] = "a very long string is right here"',
+                     '[2] = "b"',
+                     '[3] = "c"'])
+        self.expect(
+            "p ss",
+            substrs=["size=4",
+                     '[0] = "a"',
+                     '[1] = "a very long string is right here"',
+                     '[2] = "b"',
+                     '[3] = "c"'])
+        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"'])
+
+    @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_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.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)
+
+

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;
+}