This revision was automatically updated to reflect the committed changes.
Closed by commit rL249597: [lldb-mi] Fix evaluation of strings containing
characters from non-ascii range (authored by dperchik).
Changed prior to commit:
http://reviews.llvm.org/D13058?vs=36341&id=36782#toc
Repository:
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
lldb-com
ki.stfu added a comment.
In http://reviews.llvm.org/D13058#258903, @dawn wrote:
> This patch is good to commit now right? It's not marked "accepted" for some
> reason.
It's not "accepted" because @clayborg rejected this CL and hasn't changed his
decision. But I think he doesn't mind if you w
evgeny777 added a comment.
Thanks for a good point.
MI formatting refactoring will likely come soon as a separate review
http://reviews.llvm.org/D13058
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/l
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.
>
>
> http://reviews.llvm.org/D
granata.enrico added a comment.
I Thought I had marked it as good to go.
My bad if I have not.
- Enrico
http://reviews.llvm.org/D13058
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-com
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
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-
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 (lldb::SBValu
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 mo
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
test/tools/lldb-
evgeny777 added inline comments.
Comment at: include/lldb/API/SBTypeSummary.h:125
@@ +124,3 @@
+
+bool DoesPrintValue(const lldb::SBValue &value) const;
+
granata.enrico wrote:
> Can you please change this to
>
> bool
> DoesPrintValue (lldb::SBValue value
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 SBVal
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: inclu
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
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
http://lists
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
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-co
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
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
test/tools/lldb-mi
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 didn
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 CMIUti
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 = Get
evgeny777 added inline comments.
Comment at: tools/lldb-mi/MICmnLLDBUtilSBValue.h:58
@@ -57,3 +57,3 @@
bool GetCompositeValue(const bool vbPrintFieldNames, CMICmnMIValueTuple
&vwrMiValueTuple, const MIuint vnDepth = 1) const;
-
+bool TryGetValueSummary(CMIUtilString &vrV
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 t
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
test/tools/lldb-mi/variable/TestMiGdbSetShowPrint
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
test/tools/lldb-mi/variable/TestMiGdbSetShowPrint.p
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: test/tools/lldb-mi/variable/TestMiGdbSetShowPrin
evgeny777 added a comment.
Yes I know about GetValue() and stuff
Actually I implemented new method, because I needed
TypeSummaryImpl::DoesPrintValue().
In lldb when you evaluate char*, you get both value and summary, for example:
0xdeadbeef "hello"
But when you evaluate char[] array, you get ju
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.
ht
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
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 to
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 @@
ki.stfu added a subscriber: lldb-commits.
ki.stfu added a comment.
Please always add lldb-commits to subscribers
http://reviews.llvm.org/D13058
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/
32 matches
Mail list logo