I'll figure out how to port lldb's use over to yaml
On Fri, Sep 9, 2016 at 7:31 PM Mike Aizatsky wrote:
> Oh, that's great! Thanks for pointing this out. I'll just upload the
> cleaned up version in case lldb/ folks would like it. I'll figure out how
> to use our YAML library.
>
> On Fri, Sep 9,
On Fri, Sep 9, 2016 at 7:31 PM Mike Aizatsky wrote:
> aizatsky abandoned this revision.
>
>
> Comment at: include/llvm/Support/JSON.h:207
> @@ +206,3 @@
> +private:
> + typedef std::map Map;
> + typedef Map::iterator Iterator;
>
> zturner wrote:
> > `DenseMap`
aizatsky abandoned this revision.
Comment at: include/llvm/Support/JSON.h:207
@@ +206,3 @@
+private:
+ typedef std::map Map;
+ typedef Map::iterator Iterator;
zturner wrote:
> `DenseMap` seems like a better choice here.
DenseMap doesn't seem to be defined for st
Oh, that's great! Thanks for pointing this out. I'll just upload the
cleaned up version in case lldb/ folks would like it. I'll figure out how
to use our YAML library.
On Fri, Sep 9, 2016 at 6:48 PM Chandler Carruth wrote:
> chandlerc added a comment.
>
> In https://reviews.llvm.org/D24369#53847
aizatsky updated this revision to Diff 70946.
aizatsky marked 10 inline comments as done.
aizatsky added a comment.
cleanup
https://reviews.llvm.org/D24369
Files:
include/llvm/Support/JSON.h
lib/Support/CMakeLists.txt
lib/Support/JSON.cpp
unittests/Support/CMakeLists.txt
unittests/Sup
I had no idea yaml was a superset of json. Makes it an easy decision
On Fri, Sep 9, 2016 at 6:48 PM Chandler Carruth wrote:
> chandlerc added a comment.
>
> In https://reviews.llvm.org/D24369#538472, @zturner wrote:
>
> > In https://reviews.llvm.org/D24369#538422, @aizatsky wrote:
> >
> > > > I'm
chandlerc added a comment.
In https://reviews.llvm.org/D24369#538472, @zturner wrote:
> In https://reviews.llvm.org/D24369#538422, @aizatsky wrote:
>
> > > I'm a bit surprised there's not already a json parser in LLVM. Maybe
> > > there's a reason for it. I think this CL needs a few more revie
zturner added inline comments.
Comment at: include/llvm/Support/JSON.h:27
@@ +26,3 @@
+public:
+ typedef std::shared_ptr SP;
+ enum class Kind { String, Number, True, False, Null, Object, Array };
I scanned the entire patch and I'm not sure I understand the need
aizatsky updated this revision to Diff 70879.
aizatsky added a comment.
remove .clang-tidy
https://reviews.llvm.org/D24369
Files:
include/llvm/Support/JSON.h
lib/Support/CMakeLists.txt
lib/Support/JSON.cpp
unittests/Support/CMakeLists.txt
unittests/Support/JSONTest.cpp
Index: unittes
aizatsky added a comment.
In https://reviews.llvm.org/D24369#538472, @zturner wrote:
> In https://reviews.llvm.org/D24369#538422, @aizatsky wrote:
>
> > In https://reviews.llvm.org/D24369#538213, @zturner wrote:
> >
> > > Having a JSON parser in LLVM seems like a reasonable idea, but I'm afraid
aizatsky updated this revision to Diff 70877.
aizatsky added a comment.
- getting rid of StringExtractor
- renaming methods to lowerCase.
https://reviews.llvm.org/D24369
Files:
.clang-tidy
include/llvm/Support/JSON.h
lib/Support/CMakeLists.txt
lib/Support/JSON.cpp
unittests/Support/CM
aizatsky added a comment.
In https://reviews.llvm.org/D24369#538213, @zturner wrote:
> Having a JSON parser in LLVM seems like a reasonable idea, but I'm afraid the
> JSON parser that currently exists in LLDB won't do. As Pavel mentioned, the
> code has a definite LLDB feel to it, but more imp
clayborg added a comment.
I like the idea of moving this down into LLVM. We should fully LLVM-ize the
code on the first pass though and take the LLDB style out of it.
https://reviews.llvm.org/D24369
___
lldb-commits mailing list
lldb-commits@lists.
zturner added a comment.
In https://reviews.llvm.org/D24369#538422, @aizatsky wrote:
> In https://reviews.llvm.org/D24369#538213, @zturner wrote:
>
> > Having a JSON parser in LLVM seems like a reasonable idea, but I'm afraid
> > the JSON parser that currently exists in LLDB won't do. As Pavel
It just occurred to me that clang-tidy parses json, and I believe also
rolls its own json parser. Assuming nobody objects to having a json parser
in llvm, i think the clang-tidy one would be the one to use as a starting
point (assuming it's general enough), then we could delete lldb's
On Fri, Sep
zturner added reviewers: dblaikie, chandlerc.
zturner added a comment.
Having a JSON parser in LLVM seems like a reasonable idea, but I'm afraid the
JSON parser that currently exists in LLDB won't do. As Pavel mentioned, the
code has a definite LLDB feel to it, but more importantly it depends o
labath added subscribers: zturner, lldb-commits, clayborg.
labath added a comment.
I am cc'ing a bunch of people to make sure we don't do this behind anyone's
back. I don't know whether you discussed this with anyone on lldb side (it's
certainly new to me), but I think it deserves a wider audien
17 matches
Mail list logo
