[Lldb-commits] [PATCH] D13657: [lldb] char summary provider

[Eugene Leviant via lldb-commits]

evgeny777 added a comment.

One question: CreateWithCallback and CreateWithSummaryString do not require Python support and can be used with LLDB_DISABLE_PYTHON. May be it makes sense to remove conditional compilation, like it is done here:

http://reviews.llvm.org/D13577


================
Comment at: source/API/SBTypeSummary.cpp:155
@@ +154,3 @@
+                   new CXXFunctionSummaryFormat(options, 
+                       [cb] (ValueObject& valobj, Stream& stm, const TypeSummaryOptions& opt) -> bool {
+                            SBStream stream;
----------------
granata.enrico wrote:
> Should we check for cb != null here?
May be assert(cb) ?

================
Comment at: source/API/SBTypeSummary.cpp:157
@@ +156,3 @@
+                            SBStream stream;
+                            if (!cb(valobj.GetSP(), &opt, stream))
+                                return false;
----------------
granata.enrico wrote:
> I assume you are essentially relying on the SBValue constructor that takes a ValueObjectSP here, right?
> And similarly for the SummaryOptions?
You're right - implicit construction here

================
Comment at: source/API/SBTypeSummary.cpp:162
@@ +161,3 @@
+                       },
+                       "SBTypeSummary formatter callbacxk"
+                   )
----------------
granata.enrico wrote:
> Any reason not to let people submit their own name/description for the summary formatter here?
Probably it makes sense adding extra parameter to CreateWithCallback()


http://reviews.llvm.org/D13657