[Lldb-commits] [PATCH] D158010: [lldb] Allow synthetic providers in C++ and fix linking problems

[walter erquinigo via Phabricator via lldb-commits]

wallace added inline comments.


================
Comment at: lldb/source/Commands/CommandObjectType.cpp:2174
 
-#if LLDB_ENABLE_PYTHON
-
----------------
wallace wrote:
> electriclilies wrote:
> > jingham wrote:
> > > electriclilies wrote:
> > > > rriddle wrote:
> > > > > Why is this dropped?
> > > > 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. 
> > > 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.
> > 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. 
> @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?
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? 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158010/new/

https://reviews.llvm.org/D158010