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

[Enrico Granata via lldb-commits]

granata.enrico added inline comments.

================
Comment at: source/API/SBTypeSummary.cpp:155
@@ +154,3 @@
+                   new CXXFunctionSummaryFormat(options, 
+                       [cb] (ValueObject& valobj, Stream& stm, const TypeSummaryOptions& opt) -> bool {
+                            SBStream stream;
----------------
evgeny777 wrote:
> granata.enrico wrote:
> > Should we check for cb != null here?
> May be assert(cb) ?
No, I would rather much us create an hollow SBTypeSummary (with a TypeSummaryImplSP that points to nullptr). Then you would get an invalid one (IsValid() == false) but not cause a crash

================
Comment at: source/API/SBTypeSummary.cpp:157
@@ +156,3 @@
+                            SBStream stream;
+                            if (!cb(valobj.GetSP(), &opt, stream))
+                                return false;
----------------
evgeny777 wrote:
> 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
Sorry to nitpick, but is there any advantage to not using explicit construction here?


http://reviews.llvm.org/D13657