[Lldb-commits] [PATCH] D13058: LLDB-MI: Bug when evaluating strings containing characters from non-ascii range

[Eugene Leviant via lldb-commits]

evgeny777 added inline comments.

================
Comment at: tools/lldb-mi/MICmnLLDBUtilSBValue.cpp:372
@@ +371,3 @@
+CMIUtilString
+CMICmnLLDBUtilSBValue::GetValueSummary() const
+{
----------------
granata.enrico wrote:
> 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.
Short answer:
Yes types without summary work, please look at CMICmnLLDBUtilSBValue::GetSimpleValue() function 

Longer anser:
This patch only addresses specific issues
- Correct UTF-16/32 handling for characters and strings
- Correct handling of composite types, like std::string

The patch logic is: if summary is available use it (optionally combined with value). If it is not then fall back to MI default formatting (see ConvertToPrintableASCII for example). 
For other simple types, like int and float GetValueSummary() is never called (MI calls GetValue() instead)

Probably refactoring MI will provide better code, but this looks like to be out of this patch scope


http://reviews.llvm.org/D13058