Unify libstdcpp and libcxx formatters for `std::optional`

[zrthxn]

Squash many commits

[wallace]

back to your queue until you get things working

[zrthxn]

Added libcxx support

[wallace]

good job! Only some cosmetic changes are needed and you also need to delete the python code

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

775–779

remove this

916–917

same here

939–940

same

lldb/source/Plugins/Language/CPlusPlus/Generic.h

2–3

make this one line (=80 chars)

11–12

this shouldn't be a comment, and use

LLDB_SOURCE_PLUGINS_LANGUAGE_CPLUSPLUS_GENERIC_H

27

LLDB_SOURCE_PLUGINS_LANGUAGE_CPLUSPLUS_GENERIC_H

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

2–3

80 chars

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

0

delete this entire file

[zrthxn]

Removed old/dead code and commented out sections

[zrthxn]

Cosmetic changes and incorporate requested changes from review

[wallace]

great job!

[wallace]

committed as cfb075089128b2e5918afd0ce21ec10bf455e5ab

lldb/source/Plugins/Language/CPlusPlus/CMakeLists.txt

 add_lldb_library(lldbPluginCPlusPlusLanguage PLUGIN
   BlockPointer.cpp
   CPlusPlusLanguage.cpp
   CPlusPlusNameParser.cpp
   CxxStringTypes.cpp
   GenericBitset.cpp
+  GenericOptional.cpp
   LibCxx.cpp
   LibCxxAtomic.cpp
   LibCxxInitializerList.cpp
   LibCxxList.cpp
   LibCxxMap.cpp
-  LibCxxOptional.cpp
   LibCxxQueue.cpp
   LibCxxTuple.cpp
   LibCxxUnorderedMap.cpp
   LibCxxVariant.cpp
   LibCxxVector.cpp
   LibStdcpp.cpp
   LibStdcppTuple.cpp
   LibStdcppUniquePointer.cpp

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

 #include "lldb/Symbol/SymbolFile.h"
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/RegularExpression.h"
 
 #include "BlockPointer.h"
 #include "CPlusPlusNameParser.h"
 #include "CxxStringTypes.h"
+#include "Generic.h"
 #include "LibCxx.h"
 #include "LibCxxAtomic.h"
 #include "LibCxxVariant.h"
 #include "LibStdcpp.h"
 #include "MSVCUndecoratedNameParser.h"
 
 using namespace lldb;
 using namespace lldb_private;
static void LoadLibCxxFormatters(lldb::TypeCategoryImplSP cpp_category_sp) {
   AddCXXSynthetic(cpp_category_sp, LibcxxQueueFrontEndCreator,
                   "libc++ std::queue synthetic children",
                   ConstString("^std::__[[:alnum:]]+::queue<.+>(( )?&)?$"),
                   stl_synth_flags, true);
   AddCXXSynthetic(cpp_category_sp, LibcxxTupleFrontEndCreator,
                   "libc++ std::tuple synthetic children",
                   ConstString("^std::__[[:alnum:]]+::tuple<.*>(( )?&)?$"),
                   stl_synth_flags, true);
-  AddCXXSynthetic(cpp_category_sp, LibcxxOptionalFrontEndCreator,
+  AddCXXSynthetic(cpp_category_sp, LibcxxOptionalSyntheticFrontEndCreator,
                   "libc++ std::optional synthetic children",
                   ConstString("^std::__[[:alnum:]]+::optional<.+>(( )?&)?$"),
                   stl_synth_flags, true);
   AddCXXSynthetic(cpp_category_sp, LibcxxVariantFrontEndCreator,
                   "libc++ std::variant synthetic children",
                   ConstString("^std::__[[:alnum:]]+::variant<.+>(( )?&)?$"),
                   stl_synth_flags, true);
   AddCXXSynthetic(
AddCXXSummary(cpp_category_sp, LibcxxContainerSummaryProvider,
                 "libc++ std::tuple summary provider",
                 ConstString("^std::__[[:alnum:]]+::tuple<.*>(( )?&)?$"),
                 stl_summary_flags, true);
   AddCXXSummary(cpp_category_sp,
                 lldb_private::formatters::LibCxxAtomicSummaryProvider,
                 "libc++ std::atomic summary provider",
                 ConstString("^std::__[[:alnum:]]+::atomic<.+>$"),
                 stl_summary_flags, true);
   AddCXXSummary(cpp_category_sp,
-                lldb_private::formatters::LibcxxOptionalSummaryProvider,
-                "libc++ std::optional summary provider",
-                ConstString("^std::__[[:alnum:]]+::optional<.+>(( )?&)?$"),
-                stl_summary_flags, true);
-  AddCXXSummary(cpp_category_sp,
                 lldb_private::formatters::LibcxxVariantSummaryProvider,
                 "libc++ std::variant summary provider",
                 ConstString("^std::__[[:alnum:]]+::variant<.+>(( )?&)?$"),
                 stl_summary_flags, true);
 
   stl_summary_flags.SetSkipPointers(true);
 
   AddCXXSummary(cpp_category_sp,
                 lldb_private::formatters::LibcxxSmartPointerSummaryProvider,
                 "libc++ std::shared_ptr summary provider",
                 ConstString("^std::__[[:alnum:]]+::shared_ptr<.+>(( )?&)?$"),
                 stl_summary_flags, true);
cpp_category_sp->GetRegexTypeSyntheticsContainer()->Add(
       RegularExpression("^std::set<.+> >(( )?&)?$"),
       SyntheticChildrenSP(new ScriptedSyntheticChildren(
           stl_deref_flags,
           "lldb.formatters.cpp.gnu_libstdcpp.StdMapLikeSynthProvider")));
   cpp_category_sp->GetRegexTypeSyntheticsContainer()->Add(
       RegularExpression("^std::multimap<.+> >(( )?&)?$"),
       SyntheticChildrenSP(new ScriptedSyntheticChildren(
           stl_deref_flags,
           "lldb.formatters.cpp.gnu_libstdcpp.StdMapLikeSynthProvider")));
   cpp_category_sp->GetRegexTypeSyntheticsContainer()->Add(
-      RegularExpression("^std::optional<.+>(( )?&)?$"),
-      SyntheticChildrenSP(new ScriptedSyntheticChildren(
-          stl_synth_flags,
-          "lldb.formatters.cpp.gnu_libstdcpp.StdOptionalSynthProvider")));
-  cpp_category_sp->GetRegexTypeSyntheticsContainer()->Add(
       RegularExpression("^std::multiset<.+> >(( )?&)?$"),
       SyntheticChildrenSP(new ScriptedSyntheticChildren(
           stl_deref_flags,
           "lldb.formatters.cpp.gnu_libstdcpp.StdMapLikeSynthProvider")));
   cpp_category_sp->GetRegexTypeSyntheticsContainer()->Add(
       RegularExpression("^std::unordered_(multi)?(map|set)<.+> >$"),
       SyntheticChildrenSP(new ScriptedSyntheticChildren(
           stl_deref_flags,
           "lldb.formatters.cpp.gnu_libstdcpp.StdUnorderedMapSynthProvider")));
   cpp_category_sp->GetRegexTypeSyntheticsContainer()->Add(
       RegularExpression("^std::(__cxx11::)?list<.+>(( )?&)?$"),
       SyntheticChildrenSP(new ScriptedSyntheticChildren(
           stl_deref_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::optional<.+>(( )?&)?$"),
-      TypeSummaryImplSP(new ScriptSummaryFormat(
-          stl_summary_flags,
-          "lldb.formatters.cpp.gnu_libstdcpp.StdOptionalSummaryProvider")));
-  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(
AddCXXSynthetic(
       stl_synth_flags, true);
 
   AddCXXSynthetic(
       cpp_category_sp,
       lldb_private::formatters::LibStdcppBitsetSyntheticFrontEndCreator,
       "std::bitset synthetic child", ConstString("^std::bitset<.+>(( )?&)?$"),
       stl_deref_flags, true);
 
+  AddCXXSynthetic(
+      cpp_category_sp,
+      lldb_private::formatters::LibStdcppOptionalSyntheticFrontEndCreator,
+      "std::optional synthetic child",
+      ConstString("^std::optional<.+>(( )?&)?$"), stl_deref_flags, true);
+
   AddCXXSummary(cpp_category_sp,
                 lldb_private::formatters::LibStdcppUniquePointerSummaryProvider,
                 "libstdc++ std::unique_ptr summary provider",
                 ConstString("^std::unique_ptr<.+>(( )?&)?$"), stl_summary_flags,
                 true);
   AddCXXSummary(cpp_category_sp,
                 lldb_private::formatters::LibStdcppSmartPointerSummaryProvider,
                 "libstdc++ std::shared_ptr summary provider",
                 ConstString("^std::shared_ptr<.+>(( )?&)?$"), stl_summary_flags,
                 true);
   AddCXXSummary(cpp_category_sp,
                 lldb_private::formatters::LibStdcppSmartPointerSummaryProvider,
                 "libstdc++ std::weak_ptr summary provider",
                 ConstString("^std::weak_ptr<.+>(( )?&)?$"), stl_summary_flags,
                 true);
+  AddCXXSummary(
+      cpp_category_sp, lldb_private::formatters::GenericOptionalSummaryProvider,
+      "generic std::optional summary provider",
+      ConstString("^std::optional<.+>(( )?&)?$"), stl_summary_flags, true);
 }
 
 static void LoadSystemFormatters(lldb::TypeCategoryImplSP cpp_category_sp) {
   if (!cpp_category_sp)
     return;
 
   TypeSummaryImpl::Flags string_flags;
   string_flags.SetCascades(true)

lldb/source/Plugins/Language/CPlusPlus/Generic.h

+//===-- LibCxx.h -----------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_SOURCE_PLUGINS_LANGUAGE_CPLUSPLUS_GENERIC_H
+#define LLDB_SOURCE_PLUGINS_LANGUAGE_CPLUSPLUS_GENERIC_H
+
+#include "lldb/Core/ValueObject.h"
+#include "lldb/DataFormatters/TypeSummary.h"
+#include "lldb/Utility/Stream.h"
+
+namespace lldb_private {
+namespace formatters {
+
+bool GenericOptionalSummaryProvider(ValueObject &valobj, Stream &stream,
+                                    const TypeSummaryOptions &options);
+
+} // namespace formatters
+} // namespace lldb_private
+
+#endif // LLDB_SOURCE_PLUGINS_LANGUAGE_CPLUSPLUS_GENERIC_H
 No newline at end of file

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

+//===-- GenericOptional.cpp ----------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===---------------------------------------------------------------------===//
+
+#include "Generic.h"
+#include "LibCxx.h"
+#include "LibStdcpp.h"
+#include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
+#include "lldb/DataFormatters/FormattersHelpers.h"
+#include "lldb/Target/Target.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+bool lldb_private::formatters::GenericOptionalSummaryProvider(
+    ValueObject &valobj, Stream &stream, const TypeSummaryOptions &options) {
+  llvm::StringRef hasvalue_as_cstring(valobj.GetNumChildren() == 0 ? "false"
+                                                                   : "true");
+
+  stream.Printf(" Has Value=%s ", hasvalue_as_cstring.data());
+
+  return true;
+}
+
+// Synthetic Children Provider
+namespace {
+
+class GenericOptionalFrontend : public SyntheticChildrenFrontEnd {
+public:
+  enum class StdLib {
+    LibCxx,
+    LibStdcpp,
+  };
+
+  GenericOptionalFrontend(ValueObject &valobj, StdLib stdlib);
+
+  size_t GetIndexOfChildWithName(ConstString name) override {
+    return formatters::ExtractIndexFromString(name.GetCString());
+  }
+
+  bool MightHaveChildren() override { return true; }
+  size_t CalculateNumChildren() override { return m_has_value ? 1U : 0U; }
+
+  ValueObjectSP GetChildAtIndex(size_t idx) override;
+  bool Update() override;
+
+private:
+  bool m_has_value = false;
+  StdLib m_stdlib;
+};
+
+} // namespace
+
+GenericOptionalFrontend::GenericOptionalFrontend(ValueObject &valobj,
+                                                 StdLib stdlib)
+    : SyntheticChildrenFrontEnd(valobj), m_stdlib(stdlib) {
+  if (auto target_sp = m_backend.GetTargetSP()) {
+    Update();
+  }
+}
+
+bool GenericOptionalFrontend::Update() {
+  ValueObjectSP engaged_sp;
+
+  if (m_stdlib == StdLib::LibCxx)
+    engaged_sp =
+        m_backend.GetChildMemberWithName(ConstString("__engaged_"), true);
+  else if (m_stdlib == StdLib::LibStdcpp)
+    engaged_sp =
+        m_backend.GetChildMemberWithName(ConstString("_M_payload"), true)
+            ->GetChildMemberWithName(ConstString("_M_engaged"), true);
+
+  if (!engaged_sp)
+    return false;
+
+  // _M_engaged/__engaged is a bool flag and is true if the optional contains a
+  // value. Converting it to unsigned gives us a size of 1 if it contains a
+  // value and 0 if not.
+  m_has_value = engaged_sp->GetValueAsUnsigned(0) != 0;
+
+  return false;
+}
+
+ValueObjectSP GenericOptionalFrontend::GetChildAtIndex(size_t _idx) {
+  if (!m_has_value)
+    return ValueObjectSP();
+
+  ValueObjectSP val_sp;
+
+  if (m_stdlib == StdLib::LibCxx)
+    // __val_ contains the underlying value of an optional if it has one.
+    // Currently because it is part of an anonymous union
+    // GetChildMemberWithName() does not peer through and find it unless we are
+    // at the parent itself. We can obtain the parent through __engaged_.
+    val_sp = m_backend.GetChildMemberWithName(ConstString("__engaged_"), true)
+                 ->GetParent()
+                 ->GetChildAtIndex(0, true)
+                 ->GetChildMemberWithName(ConstString("__val_"), true);
+  else if (m_stdlib == StdLib::LibStdcpp)
+    // _M_value contains the underlying value of an optional if it has one.
+    val_sp = m_backend.GetChildMemberWithName(ConstString("_M_payload"), true)
+                 ->GetChildAtIndex(0, true)
+                 ->GetChildMemberWithName(ConstString("_M_payload"), true)
+                 ->GetChildMemberWithName(ConstString("_M_value"), true);
+
+  if (!val_sp)
+    return ValueObjectSP();
+
+  CompilerType holder_type = val_sp->GetCompilerType();
+
+  if (!holder_type)
+    return ValueObjectSP();
+
+  return val_sp->Clone(ConstString("Value"));
+}
+
+SyntheticChildrenFrontEnd *
+formatters::LibStdcppOptionalSyntheticFrontEndCreator(
+    CXXSyntheticChildren *, lldb::ValueObjectSP valobj_sp) {
+  if (valobj_sp)
+    return new GenericOptionalFrontend(
+        *valobj_sp, GenericOptionalFrontend::StdLib::LibStdcpp);
+  return nullptr;
+}
+
+SyntheticChildrenFrontEnd *formatters::LibcxxOptionalSyntheticFrontEndCreator(
+    CXXSyntheticChildren *, lldb::ValueObjectSP valobj_sp) {
+  if (valobj_sp)
+    return new GenericOptionalFrontend(*valobj_sp,
+                                       GenericOptionalFrontend::StdLib::LibCxx);
+  return nullptr;
+}

lldb/source/Plugins/Language/CPlusPlus/LibCxx.h

 
 SyntheticChildrenFrontEnd *LibcxxQueueFrontEndCreator(CXXSyntheticChildren *,
                                                       lldb::ValueObjectSP);
 
 SyntheticChildrenFrontEnd *LibcxxTupleFrontEndCreator(CXXSyntheticChildren *,
                                                       lldb::ValueObjectSP);
 
 SyntheticChildrenFrontEnd *
-LibcxxOptionalFrontEndCreator(CXXSyntheticChildren *,
+LibcxxOptionalSyntheticFrontEndCreator(CXXSyntheticChildren *,
                                        lldb::ValueObjectSP valobj_sp);
 
 SyntheticChildrenFrontEnd *
 LibcxxVariantFrontEndCreator(CXXSyntheticChildren *,
                              lldb::ValueObjectSP valobj_sp);
 
 } // namespace formatters
 } // namespace lldb_private
 
 #endif // LLDB_SOURCE_PLUGINS_LANGUAGE_CPLUSPLUS_LIBCXX_H

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

-//===-- LibCxxOptional.cpp ------------------------------------------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#include "LibCxx.h"
-#include "lldb/DataFormatters/FormattersHelpers.h"
-
-using namespace lldb;
-using namespace lldb_private;
-
-namespace {
-
-class OptionalFrontEnd : public SyntheticChildrenFrontEnd {
-public:
-  OptionalFrontEnd(ValueObject &valobj) : SyntheticChildrenFrontEnd(valobj) {
-    Update();
-  }
-
-  size_t GetIndexOfChildWithName(ConstString name) override {
-    return formatters::ExtractIndexFromString(name.GetCString());
-  }
-
-  bool MightHaveChildren() override { return true; }
-  bool Update() override;
-  size_t CalculateNumChildren() override { return m_has_value ? 1U : 0U; }
-  ValueObjectSP GetChildAtIndex(size_t idx) override;
-
-private:
-  /// True iff the option contains a value.
-  bool m_has_value = false;
-};
-} // namespace
-
-bool OptionalFrontEnd::Update() {
-  ValueObjectSP engaged_sp(
-      m_backend.GetChildMemberWithName(ConstString("__engaged_"), true));
-
-  if (!engaged_sp)
-    return false;
-
-  // __engaged_ is a bool flag and is true if the optional contains a value.
-  // Converting it to unsigned gives us a size of 1 if it contains a value
-  // and 0 if not.
-  m_has_value = engaged_sp->GetValueAsUnsigned(0) != 0;
-
-  return false;
-}
-
-ValueObjectSP OptionalFrontEnd::GetChildAtIndex(size_t idx) {
-  if (!m_has_value)
-    return ValueObjectSP();
-
-  // __val_ contains the underlying value of an optional if it has one.
-  // Currently because it is part of an anonymous union GetChildMemberWithName()
-  // does not peer through and find it unless we are at the parent itself.
-  // We can obtain the parent through __engaged_.
-  ValueObjectSP val_sp(
-      m_backend.GetChildMemberWithName(ConstString("__engaged_"), true)
-          ->GetParent()
-          ->GetChildAtIndex(0, true)
-          ->GetChildMemberWithName(ConstString("__val_"), true));
-
-  if (!val_sp)
-    return ValueObjectSP();
-
-  CompilerType holder_type = val_sp->GetCompilerType();
-
-  if (!holder_type)
-    return ValueObjectSP();
-
-  return val_sp->Clone(ConstString("Value"));
-}
-
-SyntheticChildrenFrontEnd *
-formatters::LibcxxOptionalFrontEndCreator(CXXSyntheticChildren *,
-                                          lldb::ValueObjectSP valobj_sp) {
-  if (valobj_sp)
-    return new OptionalFrontEnd(*valobj_sp);
-  return nullptr;
-}

lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.h

 LibStdcppTupleSyntheticFrontEndCreator(CXXSyntheticChildren *,
                                        lldb::ValueObjectSP);
 
 SyntheticChildrenFrontEnd *
 LibStdcppBitsetSyntheticFrontEndCreator(CXXSyntheticChildren *,
                                         lldb::ValueObjectSP);
 
 SyntheticChildrenFrontEnd *
+LibStdcppOptionalSyntheticFrontEndCreator(CXXSyntheticChildren *,
+                                          lldb::ValueObjectSP);
+
+SyntheticChildrenFrontEnd *
 LibStdcppVectorIteratorSyntheticFrontEndCreator(CXXSyntheticChildren *,
                                                 lldb::ValueObjectSP);
 
 SyntheticChildrenFrontEnd *
 LibStdcppSharedPtrSyntheticFrontEndCreator(CXXSyntheticChildren *,
                                            lldb::ValueObjectSP);
 
 SyntheticChildrenFrontEnd *
 LibStdcppUniquePtrSyntheticFrontEndCreator(CXXSyntheticChildren *,
                                            lldb::ValueObjectSP);
 
 } // namespace formatters
 } // namespace lldb_private
 
 #endif // LLDB_SOURCE_PLUGINS_LANGUAGE_CPLUSPLUS_LIBSTDCPP_H