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

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Back from vacation, sorry for the delay. Looks good if Enrico is happy. http://reviews.llvm.org/D13058 ___ lldb-commits mailing list

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

evgeny777 updated this revision to Diff 36341. evgeny777 added a comment. Changed DoesPrintValue() function prototype http://reviews.llvm.org/D13058 Files: include/lldb/API/SBTypeSummary.h source/API/SBTypeSummary.cpp test/tools/lldb-mi/variable/TestMiGdbSetShowPrint.py

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

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

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

granata.enrico accepted this revision. granata.enrico added a comment. 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

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

dawn added a comment. This patch is good to commit now right? It's not marked "accepted" for some reason. http://reviews.llvm.org/D13058 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

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

I Thought I had marked it as good to go. My bad if I have not. - Enrico Sent from my iPhone > On Oct 2, 2015, at 1:12 PM, Dawn Perchik wrote: > > dawn added a comment. > > This patch is good to commit now right? It's not marked "accepted" for some > reason. > > >

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

dawn added a comment. > You can use clang-format to follow the LLDB coding style, or just do the > following: 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

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

On Sat, Sep 26, 2015 at 08:13:48AM +, Jason Molenda wrote: > NB: Greg is out for the next nine days, Enrico for the next five days. Is Enrico back now? OK to commit this if he accepts? > http://reviews.llvm.org/D13058 ___ lldb-commits mailing list

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

ki.stfu accepted this revision. ki.stfu added a comment. lgtm http://reviews.llvm.org/D13058 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

ki.stfu added a comment. But you still should get lgtm from @clayborg and @granata.enrico before landing. http://reviews.llvm.org/D13058 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

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

jasonmolenda added a subscriber: jasonmolenda. jasonmolenda added a comment. NB: Greg is out for the next nine days, Enrico for the next five days. http://reviews.llvm.org/D13058 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

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

evgeny777 added inline comments. Comment at: source/API/SBTypeSummary.cpp:290 @@ +289,3 @@ +bool +SBTypeSummary::DoesPrintValue(const SBValue& value) +{ ki.stfu wrote: > ditto I used clang-format with style file taken from lldb directory here, but formatting

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

evgeny777 updated this revision to Diff 35725. evgeny777 added a comment. Revised with changes requested by ki.stfu http://reviews.llvm.org/D13058 Files: include/lldb/API/SBTypeSummary.h source/API/SBTypeSummary.cpp test/tools/lldb-mi/variable/TestMiGdbSetShowPrint.py

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

ki.stfu added inline comments. Comment at: tools/lldb-mi/MICmnLLDBUtilSBValue.cpp:191-193 @@ -182,1 +190,5 @@ { +CMIUtilString summary; +if (TryGetValueSummary(summary)) +return summary; + evgeny777 wrote: > ki.stfu wrote: > > ``` > > const

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

ki.stfu requested changes to this revision. This revision now requires changes to proceed. Comment at: include/lldb/API/SBTypeSummary.h:125-126 @@ -124,1 +124,4 @@ + +bool +DoesPrintValue (const SBValue& value); You can use clang-format

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

evgeny777 added inline comments. Comment at: tools/lldb-mi/MICmnLLDBUtilSBValue.h:58 @@ -57,3 +57,3 @@ bool GetCompositeValue(const bool vbPrintFieldNames, CMICmnMIValueTuple , const MIuint vnDepth = 1) const; - +bool TryGetValueSummary(CMIUtilString ) const; //

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

evgeny777 added inline comments. Comment at: tools/lldb-mi/MICmnLLDBUtilSBValue.cpp:191-193 @@ -182,1 +190,5 @@ { +CMIUtilString summary; +if (TryGetValueSummary(summary)) +return summary; + ki.stfu wrote: > ``` > const CMIUtilString summary =

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

evgeny777 updated this revision to Diff 35607. evgeny777 added a comment. Revised according to suggestions from granta.enrico and ki.stfu http://reviews.llvm.org/D13058 Files: include/lldb/API/SBTypeSummary.h source/API/SBTypeSummary.cpp

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

evgeny777 updated this revision to Diff 35618. evgeny777 added a comment. Revised, removed dependencies from http://reviews.llvm.org/D13094 http://reviews.llvm.org/D13058 Files: include/lldb/API/SBTypeSummary.h source/API/SBTypeSummary.cpp

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

ki.stfu requested changes to this revision. ki.stfu added a comment. Please, next time make a patch with full context: svn diff --diff-cmd diff -x "-U" > mypatch.txt This will help us to reduce the review time. Comment at:

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

granata.enrico added a comment. 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.

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

clayborg added a comment. 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. http://reviews.llvm.org/D13058

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

granata.enrico added a subscriber: granata.enrico. granata.enrico requested changes to this revision. granata.enrico added a reviewer: granata.enrico. granata.enrico added a comment. Is there a reason to explicitly add an API to get the concatenation of value and summary? We already have APIs

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

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. You can't use std::string in the public API. Use lldb::SBStream as noted in inlined comments. Comment at: include/lldb/API/SBValue.h:132-133 @@ -131,1 +131,4