[lldb] Allow synthetic providers in C++ and fix linking problems

[electriclilies]

Summary

• Allow the definition of synthetic formatters in C++ even when LLDB is built without python scripting support.
• Fix linking problems with the CXXSyntheticChildren

[bulbazord]

Adding a few relevant reviewers :)

[rriddle]

Moving the constructor/destructor to the cpp file makes sense to remove the "undefined reference to vtable" issues that we've seen before, but I don't understand the changes to pure virtual methods or the removal of the LLDB_ENABLE_PYTHON checks. Can you elaborate more?

lldb/include/lldb/DataFormatters/TypeSynthetic.h

257–262

Why these changes?

lldb/source/Commands/CommandObjectType.cpp

2174

Why is this dropped?

[electriclilies]

lldb/include/lldb/DataFormatters/TypeSynthetic.h

257–262

I thought that this was causing linking problems but it is not, so I will revert.

lldb/source/Commands/CommandObjectType.cpp

2174

Walter and I want to use the synthetic types from C++, but right now it's only supported in Python. The motivation behind this is to make it so that we can actually use the synthetic types.

[jingham]

lldb/source/Commands/CommandObjectType.cpp

2174

Just to be clear. There are already lots of synthetic child providers implemented in C++ in in lldb already (look for CXXSyntheticChildren constructors in CPlusPlusLanguage.cpp for instance).

I think what you are saying is that you are removing the restriction that you have to BUILD lldb with Python in order to add C++ implemented summaries. If that's indeed what you are saying, then this is fine, since that seems an odd restriction...

But it would be nice to have some kind of testing that this actually works, since I don't think any of our bots build lldb w/o Python.

[electriclilies]

lldb/source/Commands/CommandObjectType.cpp

2174

Yes that's exactly it! I agree it would be nice to have a test for it but I don't know how to add it.

[electriclilies]

Fix

[electriclilies]

@jingham Any other changes I should make? Thanks!

[wallace]

lldb/source/Commands/CommandObjectType.cpp

2174

@jingham , indeed, we couldn't add buildbot tests for this they all build with python support.

However, a good compromise is to enable all these commands except for the add ones, because they have the option to use python-defined extensions. clear, delete, info and list should be fine even without python scripting support.

What do you think?

[wallace]

lldb/source/Commands/CommandObjectType.cpp

2174

After a second inspection of this diff, I want to rephrase my previous comment:

This patch is only enabling the type synthetic commands, and type synthetic add is the only one that can receive a python input. In fact, it only works with python, so @electriclilies, please keep type synthetic add guarded by #if LLDB_ENABLE_PYTHON, because it just doesn't work otherwise.

@jingham , we can't really test this in the buildbots, so, all good if Lily does some basic manual testing showing that these commands work even when the python scripting support is not enabled?

[electriclilies]

@wallace The type synthetic add command already disables itself if python is not enabled (the check is at CommandObjectType::1564), so I think there aren't any changes I need to make

[wallace]

In D158010#4617611, @electriclilies wrote:

@wallace The type synthetic add command already disables itself if python is not enabled (the check is at CommandObjectType::1564), so I think there aren't any changes I need to make

Great! Let's wait for Jim then.

[wallace]

For the sake of unblocking this diff, I'm accepting it because there's no clear actionable feedback at the moment. @electriclilies, please follow up any feedback that is submitted post-landing.

[jingham]

Sorry, I got distracted, but after the fact I also agree this is reasonable.

We might actually be able to support externally adding C++ based synthetic providers, though I think the best way to do that is to add a way to make an SBTypeSynthetic from the appropriate C++ class, and then you could make that and add it in the lldb init function in the C++ library that adds the provider.

[wallace]

add a way to make an SBTypeSynthetic from the appropriate C++ class

That's a great idea. Once we implement a couple of providers for our plugin I'll revisit the idea. Thanks!

lldb/include/lldb/DataFormatters/TypeSynthetic.h

public:
     uint32_t GetValue() { return m_flags; }
 
     void SetValue(uint32_t value) { m_flags = value; }
 
   private:
     uint32_t m_flags = lldb::eTypeOptionCascade;
   };
 
-  SyntheticChildren(const Flags &flags) : m_flags(flags) {}
+  SyntheticChildren(const Flags &flags);
 
-  virtual ~SyntheticChildren() = default;
+  virtual ~SyntheticChildren();
 
   bool Cascades() const { return m_flags.GetCascades(); }
 
   bool SkipsPointers() const { return m_flags.GetSkipPointers(); }
 
   bool SkipsReferences() const { return m_flags.GetSkipReferences(); }
 
   bool NonCacheable() const { return m_flags.GetNonCacheable(); }
-  
-  bool WantsDereference() const { return m_flags.GetFrontEndWantsDereference();} 
+  bool WantsDereference() const { return m_flags.GetFrontEndWantsDereference();}
 
   void SetCascades(bool value) { m_flags.SetCascades(value); }
 
   void SetSkipsPointers(bool value) { m_flags.SetSkipPointers(value); }
 
   void SetSkipsReferences(bool value) { m_flags.SetSkipReferences(value); }
 
   void SetNonCacheable(bool value) { m_flags.SetNonCacheable(value); }
 
   uint32_t GetOptions() { return m_flags.GetValue(); }
 
   void SetOptions(uint32_t value) { m_flags.SetValue(value); }
 
   virtual bool IsScripted() = 0;
 
   virtual std::string GetDescription() = 0;
 
   virtual SyntheticChildrenFrontEnd::AutoPointer
   GetFrontEnd(ValueObject &backend) = 0;
 
   typedef std::shared_ptr<SyntheticChildren> SharedPointer;
 
   uint32_t &GetRevision() { return m_my_revision; }
 
 protected:
   uint32_t m_my_revision = 0;
   Flags m_flags;

 };
 
 class CXXSyntheticChildren : public SyntheticChildren {
 public:
   typedef std::function<SyntheticChildrenFrontEnd *(CXXSyntheticChildren *,
                                                     lldb::ValueObjectSP)>
       CreateFrontEndCallback;
   CXXSyntheticChildren(const SyntheticChildren::Flags &flags,
-                       const char *description, CreateFrontEndCallback callback)
-      : SyntheticChildren(flags), m_create_callback(std::move(callback)),
-        m_description(description ? description : "") {}
+                       const char *description, CreateFrontEndCallback callback);
+
+  virtual ~CXXSyntheticChildren();
 
   bool IsScripted() override { return false; }
 
   std::string GetDescription() override;
 
   SyntheticChildrenFrontEnd::AutoPointer
   GetFrontEnd(ValueObject &backend) override {
     return SyntheticChildrenFrontEnd::AutoPointer(

lldb/source/Commands/CommandObjectType.cpp

 class CommandObjectTypeFilterList
     : public CommandObjectTypeFormatterList<TypeFilterImpl> {
 public:
   CommandObjectTypeFilterList(CommandInterpreter &interpreter)
       : CommandObjectTypeFormatterList(interpreter, "type filter list",
                                        "Show a list of current filters.") {}
 };
 
-#if LLDB_ENABLE_PYTHON
-
 // CommandObjectTypeSynthList
 
 class CommandObjectTypeSynthList
     : public CommandObjectTypeFormatterList<SyntheticChildren> {
 public:
   CommandObjectTypeSynthList(CommandInterpreter &interpreter)
       : CommandObjectTypeFormatterList(
             interpreter, "type synthetic list",
             "Show a list of current synthetic providers.") {}
 };
 
-#endif
-
 // CommandObjectTypeFilterDelete
 
 class CommandObjectTypeFilterDelete : public CommandObjectTypeFormatterDelete {
 public:
   CommandObjectTypeFilterDelete(CommandInterpreter &interpreter)
       : CommandObjectTypeFormatterDelete(
             interpreter, eFormatCategoryItemFilter) {}
 
   ~CommandObjectTypeFilterDelete() override = default;
 };
 
-#if LLDB_ENABLE_PYTHON
-
 // CommandObjectTypeSynthDelete
 
 class CommandObjectTypeSynthDelete : public CommandObjectTypeFormatterDelete {
 public:
   CommandObjectTypeSynthDelete(CommandInterpreter &interpreter)
       : CommandObjectTypeFormatterDelete(
             interpreter, eFormatCategoryItemSynth) {}
 
   ~CommandObjectTypeSynthDelete() override = default;
 };
 
-#endif
 
 // CommandObjectTypeFilterClear
 
 class CommandObjectTypeFilterClear : public CommandObjectTypeFormatterClear {
 public:
   CommandObjectTypeFilterClear(CommandInterpreter &interpreter)
       : CommandObjectTypeFormatterClear(interpreter, eFormatCategoryItemFilter,
                                         "type filter clear",
                                         "Delete all existing filter.") {}
 };
 
-#if LLDB_ENABLE_PYTHON
 // CommandObjectTypeSynthClear
 
 class CommandObjectTypeSynthClear : public CommandObjectTypeFormatterClear {
 public:
   CommandObjectTypeSynthClear(CommandInterpreter &interpreter)
       : CommandObjectTypeFormatterClear(
             interpreter, eFormatCategoryItemSynth, "type synthetic clear",
             "Delete all existing synthetic providers.") {}
if (interpreter && !interpreter->CheckObjectExists(function_name)) {
       return false;
     }
   }
 
   category->AddTypeSynthetic(type_name.GetStringRef(), match_type, entry);
   return true;
 }
 
-#endif
 #define LLDB_OPTIONS_type_filter_add
 #include "CommandOptions.inc"
 
 class CommandObjectTypeFilterAdd : public CommandObjectParsed {
 private:
   class CommandOptions : public Options {
     typedef std::vector<std::string> option_vector;
 
LoadSubCommand(
                     [](ValueObject &valobj) -> TypeFormatImpl::SharedPointer {
                       return valobj.GetValueFormat();
                     })));
   }
 
   ~CommandObjectTypeFormat() override = default;
 };
 
-#if LLDB_ENABLE_PYTHON
-
 class CommandObjectTypeSynth : public CommandObjectMultiword {
 public:
   CommandObjectTypeSynth(CommandInterpreter &interpreter)
       : CommandObjectMultiword(
             interpreter, "type synthetic",
             "Commands for operating on synthetic type representations.",
             "type synthetic [<sub-command-options>] ") {
     LoadSubCommand("add",
LoadSubCommand(
             [](ValueObject &valobj) -> SyntheticChildren::SharedPointer {
               return valobj.GetSyntheticChildren();
             })));
   }
 
   ~CommandObjectTypeSynth() override = default;
 };
 
-#endif
-
 class CommandObjectTypeFilter : public CommandObjectMultiword {
 public:
   CommandObjectTypeFilter(CommandInterpreter &interpreter)
       : CommandObjectMultiword(interpreter, "type filter",
                                "Commands for operating on type filters.",
                                "type filter [<sub-command-options>] ") {
     LoadSubCommand(
         "add", CommandObjectSP(new CommandObjectTypeFilterAdd(interpreter)));
CommandObjectType::CommandObjectType(CommandInterpreter &interpreter)
   LoadSubCommand("category",
                  CommandObjectSP(new CommandObjectTypeCategory(interpreter)));
   LoadSubCommand("filter",
                  CommandObjectSP(new CommandObjectTypeFilter(interpreter)));
   LoadSubCommand("format",
                  CommandObjectSP(new CommandObjectTypeFormat(interpreter)));
   LoadSubCommand("summary",
                  CommandObjectSP(new CommandObjectTypeSummary(interpreter)));
-#if LLDB_ENABLE_PYTHON
   LoadSubCommand("synthetic",
                  CommandObjectSP(new CommandObjectTypeSynth(interpreter)));
-#endif
   LoadSubCommand("lookup",
                  CommandObjectSP(new CommandObjectTypeLookup(interpreter)));
 }
 
 CommandObjectType::~CommandObjectType() = default;

lldb/source/Core/ValueObject.cpp

bool ValueObject::UpdateFormatsIfNeeded() {
 
   if ((m_last_format_mgr_revision != DataVisualization::GetCurrentRevision())) {
     m_last_format_mgr_revision = DataVisualization::GetCurrentRevision();
     any_change = true;
 
     SetValueFormat(DataVisualization::GetFormat(*this, eNoDynamicValues));
     SetSummaryFormat(
         DataVisualization::GetSummaryFormat(*this, GetDynamicValueType()));
-#if LLDB_ENABLE_PYTHON
     SetSyntheticChildren(
         DataVisualization::GetSyntheticChildren(*this, GetDynamicValueType()));
-#endif
   }
 
   return any_change;
 }
 
 void ValueObject::SetNeedsUpdate() {
   m_update_point.SetNeedsUpdate();
   // We have to clear the value string here so ConstResult children will notice
bool ValueObject::HasSpecialPrintableRepresentation(
   }
   return false;
 }
 
 bool ValueObject::DumpPrintableRepresentation(
     Stream &s, ValueObjectRepresentationStyle val_obj_display,
     Format custom_format, PrintableRepresentationSpecialCases special,
     bool do_dump_error) {
-    
   // If the ValueObject has an error, we might end up dumping the type, which
   // is useful, but if we don't even have a type, then don't examine the object
   // further as that's not meaningful, only the error is.
   if (m_error.Fail() && !GetCompilerType().IsValid()) {
     if (do_dump_error)
       s.Printf("<%s>", m_error.AsCString());
     return false;
   }

 }
 
 ValueObjectSP ValueObject::DoCast(const CompilerType &compiler_type) {
     return ValueObjectCast::Create(*this, GetName(), compiler_type);
 }
 
 ValueObjectSP ValueObject::Cast(const CompilerType &compiler_type) {
   // Only allow casts if the original type is equal or larger than the cast
-  // type.  We don't know how to fetch more data for all the ConstResult types, 
+  // type.  We don't know how to fetch more data for all the ConstResult types,
   // so we can't guarantee this will work:
   Status error;
   CompilerType my_type = GetCompilerType();
 
-  ExecutionContextScope *exe_scope 
+  ExecutionContextScope *exe_scope
       = ExecutionContext(GetExecutionContextRef())
           .GetBestExecutionContextScope();
-  if (compiler_type.GetByteSize(exe_scope) 
+  if (compiler_type.GetByteSize(exe_scope)
       <= GetCompilerType().GetByteSize(exe_scope)) {
         return DoCast(compiler_type);
   }
   error.SetErrorString("Can only cast to a type that is equal to or smaller "
                        "than the orignal type.");
 
   return ValueObjectConstResult::Create(
       ExecutionContext(GetExecutionContextRef()).GetBestExecutionContextScope(),

lldb/source/DataFormatters/TypeSynthetic.cpp

std::string TypeFilterImpl::GetDescription() {
   for (size_t i = 0; i < GetCount(); i++) {
     sstr.Printf("    %s\n", GetExpressionPathAtIndex(i));
   }
 
   sstr.Printf("}");
   return std::string(sstr.GetString());
 }
 
+SyntheticChildren::SyntheticChildren(const Flags &flags) : m_flags(flags) {}
+
+SyntheticChildren::~SyntheticChildren() = default;
+
+CXXSyntheticChildren::CXXSyntheticChildren(
+    const SyntheticChildren::Flags &flags, const char *description,
+    CreateFrontEndCallback callback)
+    : SyntheticChildren(flags), m_create_callback(std::move(callback)),
+      m_description(description ? description : "") {}
+
+CXXSyntheticChildren::~CXXSyntheticChildren() = default;
+
+bool SyntheticChildren::IsScripted() { return false; }
+
+std::string SyntheticChildren::GetDescription() { return ""; }
+
+SyntheticChildrenFrontEnd::AutoPointer
+SyntheticChildren::GetFrontEnd(ValueObject &backend) {
+  return nullptr;
+}
+
 std::string CXXSyntheticChildren::GetDescription() {
   StreamString sstr;
   sstr.Printf("%s%s%s %s", Cascades() ? "" : " (not cascading)",
               SkipsPointers() ? " (skip pointers)" : "",
               SkipsReferences() ? " (skip references)" : "",
               m_description.c_str());
 
   return std::string(sstr.GetString());