[Lldb-commits] [PATCH] D68179: [lldb] Fix JSON parser to allow empty arrays

This revision was automatically updated to reflect the committed changes. Closed by commit rG10b851434324: [lldb] Fix JSON parser to allow empty arrays (authored by tetsuo-cpp, committed by teemperor). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68

[Lldb-commits] [PATCH] D68179: [lldb] Fix JSON parser to allow empty arrays

tetsuo-cpp added a comment. Thanks for the reviews! @davide could you please commit this on my behalf? Thanks for the advice. I intend to continue working on LLDB when I can so I will spend some time to fix the test suite on my Mac. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68179/n

[Lldb-commits] [PATCH] D68179: [lldb] Fix JSON parser to allow empty arrays

labath accepted this revision. labath added a comment. I'm not sure whose approval are you waiting for, but this LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68179/new/ https://reviews.llvm.org/D68179 ___ lldb-commits mailing list ll

[Lldb-commits] [PATCH] D68179: [lldb] Fix JSON parser to allow empty arrays

davide added a comment. In D68179#1745614 , @tetsuo-cpp wrote: > Thanks for looking at this. I will need someone to commit it for me. > However, I've been having issues with the test suite on my MacBook. > `check-lldb-unit` works for me but `check-llvm`

[Lldb-commits] [PATCH] D68179: [lldb] Fix JSON parser to allow empty arrays

tetsuo-cpp marked an inline comment as done. tetsuo-cpp added a comment. Thanks for looking at this. I will need someone to commit it for me. However, I've been having issues with the test suite on my MacBook. `check-lldb-unit` works for me but `check-llvm` and `check-lldb` are hitting issues be

[Lldb-commits] [PATCH] D68179: [lldb] Fix JSON parser to allow empty arrays

tetsuo-cpp updated this revision to Diff 229291. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68179/new/ https://reviews.llvm.org/D68179 Files: lldb/tools/debugserver/source/JSON.cpp lldb/tools/debugserver/source/JSON.h lldb/unittests/debugserver/CMakeLists.txt lldb/unittests/de

[Lldb-commits] [PATCH] D68179: [lldb] Fix JSON parser to allow empty arrays

labath added a comment. Looks good to me. If I was writing the test, I'd probably just make it do a string->json->string round-trip and then verify the final string. But this is fine too... Comment at: lldb/unittests/debugserver/JSONTest.cpp:16-17 +void TestJSON(JSONValue *js

[Lldb-commits] [PATCH] D68179: [lldb] Fix JSON parser to allow empty arrays

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. Thanks for adding the tests. I took a look at the patch and it seems fine to me. Do you need somebody to commit this for you? @jingham or @labath may want to have another look. CHANGES SINCE

[Lldb-commits] [PATCH] D68179: [lldb] Fix JSON parser to allow empty arrays

tetsuo-cpp added a comment. Friendly ping. Could I please get this looked at? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68179/new/ https://reviews.llvm.org/D68179 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.l

[Lldb-commits] [PATCH] D68179: [lldb] Fix JSON parser to allow empty arrays

tetsuo-cpp added a comment. I've had a try at writing some unit tests for the JSON parser in `debugserver`, including the empty array case which I'm fixing in this patch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68179/new/ https://reviews.llvm.org/D68179 ___

[Lldb-commits] [PATCH] D68179: [lldb] Fix JSON parser to allow empty arrays

tetsuo-cpp updated this revision to Diff 228561. Herald added a subscriber: mgorny. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68179/new/ https://reviews.llvm.org/D68179 Files: lldb/tools/debugserver/source/JSON.cpp lldb/tools/debugserver/source/JSON.h lldb/unittests/debugserver

[Lldb-commits] [PATCH] D68179: [lldb] Fix JSON parser to allow empty arrays

tetsuo-cpp added a comment. Thank you for the suggestions! I'm assuming that since only the `debugserver` portion of this change is left, I should write a test specifically for that. I had a quick look and it wasn't obvious how to do that but I will spend some more time on it this weekend. @ji

[Lldb-commits] [PATCH] D68179: [lldb] Fix JSON parser to allow empty arrays

tetsuo-cpp updated this revision to Diff 222923. tetsuo-cpp added a comment. Rebased onto trunk. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68179/new/ https://reviews.llvm.org/D68179 Files: lldb/tools/debugserver/source/JSON.cpp lldb/tools/debugserver/source/JSON.h Index: lldb

Re: [Lldb-commits] [PATCH] D68179: [lldb] Fix JSON parser to allow empty arrays

debugserver doesn't use llvm classes, it's its own stand-alone thing. So debugserver wasn't included in Jonas' changes. Jim > On Oct 1, 2019, at 3:11 PM, Alex Cameron via Phabricator > wrote: > > tetsuo-cpp added a comment. > > In D68179#1690073 , @

[Lldb-commits] [PATCH] D68179: [lldb] Fix JSON parser to allow empty arrays

tetsuo-cpp added a comment. In D68179#1690073 , @JDevlieghere wrote: > Hey, I just want to give you a heads up that I'm in the process to replace > LLDB's JSON implementation with the one from LLVM. The parts in > StructuredData are already gone (r37335

[Lldb-commits] [PATCH] D68179: [lldb] Fix JSON parser to allow empty arrays

JDevlieghere added a comment. Hey, I just want to give you a heads up that I'm in the process to replace LLDB's JSON implementation with the one from LLVM. The parts in StructuredData are already gone (r373359, r373360) and I'm currently working on the other uses in LLDB, except for debugserver

[Lldb-commits] [PATCH] D68179: [lldb] Fix JSON parser to allow empty arrays

jingham added a comment. It would be good as Pavel and Davide suggest to write a test directly for the JSON parser. Doing so in the C++ Unit test seems the most convenient, but that's up to you. But it would also be good to add a test for the particular "breakpoint write" -> "breakpoint read"

[Lldb-commits] [PATCH] D68179: [lldb] Fix JSON parser to allow empty arrays

labath added a comment. In D68179#1686893 , @davide wrote: > This needs a test. You can either write a python one as the ones in > `test/testcases` or a lit-style based as the ones in `lit/`. Another option might be a c++ unit test for the JSON class (s

[Lldb-commits] [PATCH] D68179: [lldb] Fix JSON parser to allow empty arrays

davide requested changes to this revision. davide added a comment. This revision now requires changes to proceed. This needs a test. You can either write a python one as the ones in `test/testcases` or a lit-style based as the ones in `lit/`. CHANGES SINCE LAST ACTION https://reviews.llvm.org

[Lldb-commits] [PATCH] D68179: [lldb] Fix JSON parser to allow empty arrays

tetsuo-cpp updated this revision to Diff 81. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68179/new/ https://reviews.llvm.org/D68179 Files: lldb/include/lldb/Utility/JSON.h lldb/source/Utility/JSON.cpp lldb/source/Utility/StructuredData.cpp lldb/tools/debugserver/source/JSON

[Lldb-commits] [PATCH] D68179: [lldb] Fix JSON parser to allow empty arrays

tetsuo-cpp created this revision. tetsuo-cpp added a reviewer: xbolva00. tetsuo-cpp added projects: LLVM, LLDB. Herald added subscribers: lldb-commits, JDevlieghere. Bugzilla: https://bugs.llvm.org/show_bug.cgi?id=39405 alexc@kitty:~/work/wiredtiger/build_posix$ cat breakpoint.json [{"Breakpo