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

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to