granata.enrico requested changes to this revision.
granata.enrico added a comment.
This revision now requires changes to proceed.
Sorry, I didn't realize you were still waiting on me to land this patch.
I have a few extra comments before this is ready to land.
================
Comment at: include/lldb/API/SBTypeSummary.h:125
@@ +124,3 @@
+
+ bool DoesPrintValue(const lldb::SBValue &value) const;
+
----------------
Can you please change this to
bool
DoesPrintValue (lldb::SBValue value);
There is no need to pass the SBValue by reference since you're not actually
altering it - and since SBValue wraps a (glorified) pointer to a ValueObject
you're not really copying much data over. I don't see the need to classify the
function as const (only IsValid() seems to be marked thusly in that file), and
definitely no need for the SBValue to be a const
================
Comment at: source/API/SBTypeSummary.cpp:289
@@ -286,1 +288,3 @@
+bool
+SBTypeSummary::DoesPrintValue(const lldb::SBValue &value) const
----------------
Of course change this in the C++ file as well
================
Comment at: tools/lldb-mi/MICmnLLDBUtilSBValue.cpp:372
@@ +371,3 @@
+CMIUtilString
+CMICmnLLDBUtilSBValue::GetValueSummary() const
+{
----------------
I might be missing something, but how is this going to work when an SBValue has
a value but no summary (something as simple as int x = 1;)?
GetSummary() is free to return NULL, and I only see you fetching the value here
when there is a summary. Your logic should be more like
GetSummary()
if (!summary || summary && DoesPrintValue())
append value
A very long-term goal of mine is for all of this logic to exist in the
ValueObjectPrinter and clients to simply be able to ask it to print a
ValueObject according to some specification of theirs, in the middle of an
existing stream. But that is way beyond the scope of your patch, so if you just
clean up the logic here to do the right thing for values without summaries, I
am happy.
http://reviews.llvm.org/D13058
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
