Differential D13058
LLDB-MI: Bug when evaluating strings containing characters from non-ascii range Authored by evgeny777 on Sep 22 2015, 9:58 AM.
Details
In case string contains characters out of ASCII range, lldb-mi prints them as hexadecimal codes Another problem is incorrect evaluation of some composite types, like std::string, which is also fixed in a way described above (using type summary provider)
Diff Detail
Event TimelineComment Actions You can't use std::string in the public API. Use lldb::SBStream as noted in inlined comments.
Comment Actions Is there a reason to explicitly add an API to get the concatenation of value and summary? We already have APIs to retrieve the pieces: const char * GetValue (); const char * GetSummary (); We even already have an API to retrieve the summary in an SBStream: const char *
GetSummary (lldb::SBStream& stream,
lldb::SBTypeSummaryOptions& options);Is there a reason why the MI can't just call these APIs and then process the results as it sees fit? Comment Actions As Enrico stated. there is already a SBStream based way to get the summary in "const char * SBValueGetSummary (lldb::SBStream& stream, lldb::SBTypeSummaryOptions& options);" so no need to add anything as I had suggested. Comment Actions Yes I know about GetValue() and stuff 0xdeadbeef "hello" But when you evaluate char[] array, you get just "hello". Value is not printed, because DoesPrintValue() method returns false I wanted to replicate the same behavior in lldb-mi. But if adding methods to SBValue should be avoided, I'll think of some workaround inside lldb-mi. Comment Actions Gotcha! My suggestion would be to add the DoesPrintValue() logic to SBTypeSummary. I can't recall if it's there already - but if not, it would be a fine thing to add. Then the MI could use SBValue and SBTypeSummary to make that determination as it sees fit. Comment Actions Please, next time make a patch with full context: svn diff --diff-cmd diff -x "-U9999" > mypatch.txt This will help us to reduce the review time.
Comment Actions 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 Actions I am not deeply involved in MI internals, so I am going to assume you ran the test suite, and that you actually verified that types without summaries still work In general, once you opt into data formatters, you get a lot more than simply UTF handling. I understand that this is your rationale for the change, but you may want to make sure that MI still does the right thing if users define formatters for types that have nothing to do with strings (e.g. what if somebody does define a summary for int or float?) With that said, this looks reasonable, and an MI refactoring would be outside of my realm anyway. Comment Actions
So we've been telling folks to use clang-format, but it's not working correctly (my version is clang-format version 3.6.0 (217927)). It's turns: bool DoesPrintValue (lldb::SBValue value); into: bool DoesPrintValue (lldb::SBValue value); We have AlwaysBreakAfterDefinitionReturnType set to true, but that only works on function definitions. I see there is a patch for AlwaysBreakAfterDeclarationReturnType at http://reviews.llvm.org/D10370?id=27471, but it's not been accepted yet. So for now, we can't rely on clang-format for function declarations :( Comment Actions Thanks for a good point. Comment Actions It's not "accepted" because @clayborg rejected this CL and hasn't changed his decision. But I think he will not mind if you go ahead. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||