[Lldb-commits] [PATCH] D68248: [JSON] Use LLVM's library for encoding JSON

2019-10-01 Thread Jonas Devlieghere via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL373359: [JSON] Use LLVMs library for encoding JSON in 
StructuredData (authored by JDevlieghere, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D68248?vs=222509=222645#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68248/new/

https://reviews.llvm.org/D68248

Files:
  lldb/trunk/include/lldb/Core/StructuredDataImpl.h
  lldb/trunk/include/lldb/Utility/StructuredData.h
  lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/trunk/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
  lldb/trunk/source/Utility/StructuredData.cpp
  lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp

Index: lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
===
--- lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -384,9 +384,9 @@
 
   // Since the line is exceeding 80 characters.
   std::string expected_packet1 =
-  R"(jTraceStart:{"buffersize" : 8192,"metabuffersize" : 8192,"params" :)";
+  R"(jTraceStart:{"buffersize":8192,"metabuffersize":8192,"params":)";
   std::string expected_packet2 =
-  R"( {"psb" : 1,"tracetech" : "intel-pt"},"threadid" : 35,"type" : 1})";
+  R"({"psb":1,"tracetech":"intel-pt"},"threadid":35,"type":1})";
   HandlePacket(server, (expected_packet1 + expected_packet2), "1");
   ASSERT_TRUE(error.Success());
   ASSERT_EQ(result.get(), 1u);
@@ -409,8 +409,7 @@
 return client.SendStopTracePacket(trace_id, thread_id);
   });
 
-  const char *expected_packet =
-  R"(jTraceStop:{"threadid" : 35,"traceid" : 3})";
+  const char *expected_packet = R"(jTraceStop:{"threadid":35,"traceid":3})";
   HandlePacket(server, expected_packet, "OK");
   ASSERT_TRUE(result.get().Success());
 
@@ -435,8 +434,8 @@
   });
 
   std::string expected_packet1 =
-  R"(jTraceBufferRead:{"buffersize" : 32,"offset" : 0,"threadid" : 35,)";
-  std::string expected_packet2 = R"("traceid" : 3})";
+  R"(jTraceBufferRead:{"buffersize":32,"offset":0,"threadid":35,)";
+  std::string expected_packet2 = R"("traceid":3})";
   HandlePacket(server, expected_packet1+expected_packet2, "123456");
   ASSERT_TRUE(result.get().Success());
   ASSERT_EQ(buffer.size(), 3u);
@@ -467,8 +466,8 @@
   });
 
   std::string expected_packet1 =
-  R"(jTraceMetaRead:{"buffersize" : 32,"offset" : 0,"threadid" : 35,)";
-  std::string expected_packet2 = R"("traceid" : 3})";
+  R"(jTraceMetaRead:{"buffersize":32,"offset":0,"threadid":35,)";
+  std::string expected_packet2 = R"("traceid":3})";
   HandlePacket(server, expected_packet1+expected_packet2, "123456");
   ASSERT_TRUE(result.get().Success());
   ASSERT_EQ(buffer.size(), 3u);
@@ -497,11 +496,10 @@
   });
 
   const char *expected_packet =
-  R"(jTraceConfigRead:{"threadid" : 35,"traceid" : 3})";
+  R"(jTraceConfigRead:{"threadid":35,"traceid":3})";
   std::string response1 =
-  R"({"buffersize" : 8192,"params" : {"psb" : 1,"tracetech" : "intel-pt"})";
-  std::string response2 =
-  R"(],"metabuffersize" : 8192,"threadid" : 35,"type" : 1}])";
+  R"({"buffersize":8192,"params":{"psb":1,"tracetech":"intel-pt"})";
+  std::string response2 = R"(],"metabuffersize":8192,"threadid":35,"type":1}])";
   HandlePacket(server, expected_packet, response1+response2);
   ASSERT_TRUE(result.get().Success());
   ASSERT_EQ(options.getTraceBufferSize(), 8192u);
Index: lldb/trunk/source/Utility/StructuredData.cpp
===
--- lldb/trunk/source/Utility/StructuredData.cpp
+++ lldb/trunk/source/Utility/StructuredData.cpp
@@ -11,7 +11,6 @@
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/JSON.h"
 #include "lldb/Utility/Status.h"
-#include "lldb/Utility/Stream.h"
 #include "lldb/Utility/StreamString.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/MemoryBuffer.h"
@@ -21,6 +20,7 @@
 #include 
 
 using namespace lldb_private;
+using namespace llvm;
 
 // Functions that use a JSONParser to parse JSON into StructuredData
 static StructuredData::ObjectSP ParseJSONValue(JSONParser _parser);
@@ -181,98 +181,48 @@
 }
 
 void StructuredData::Object::DumpToStdout(bool pretty_print) const {
-  StreamString stream;
-  Dump(stream, pretty_print);
-  llvm::outs() << stream.GetString();
+  json::OStream stream(llvm::outs(), pretty_print ? 2 : 0);
+  Serialize(stream);
 }
 
-void StructuredData::Array::Dump(Stream , bool pretty_print) const {
-  bool first = true;
-  s << "[";
-  if (pretty_print) {
-s << "\n";
-s.IndentMore();
-  }
+void

[Lldb-commits] [PATCH] D68248: [JSON] Use LLVM's library for encoding JSON

2019-10-01 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere added a comment.

In D68248#1688975 , @vsk wrote:

> Sweet! Does this 'automatically' fix the 'llvm-argdumper has issues escaping 
> JSON-ified input' issue we discussed in person?


No, that uses the JSON class that Pavel mentioned. My hope is to remove that 
altogether in favor of the LLVM counterpart.

In D68248#1689855 , @aprantl wrote:

> Cool. I mean both are supposed to be JSON, but are we expecting any fallout 
> from a debugserver using the old library vs and lldb using the new one? I 
> suppose not..


I ran the test suite with the just-built debugserver and didn't notice any 
regressions. If this does come up we should fix the debugserver implementation.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68248/new/

https://reviews.llvm.org/D68248



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68248: [JSON] Use LLVM's library for encoding JSON

2019-10-01 Thread Adrian Prantl via Phabricator via lldb-commits 
aprantl added a comment.

Cool. I mean both are supposed to be JSON, but are we expecting any fallout 
from a debugserver using the old library vs and lldb using the new one? I 
suppose not..


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68248/new/

https://reviews.llvm.org/D68248



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68248: [JSON] Use LLVM's library for encoding JSON

2019-10-01 Thread Pavel Labath via Phabricator via lldb-commits 
labath accepted this revision.
labath added a comment.

Yay.

BTW, there's another copy of json serialization code in JSON.cpp 
(JSONValue::Write).


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68248/new/

https://reviews.llvm.org/D68248



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68248: [JSON] Use LLVM's library for encoding JSON

2019-09-30 Thread Vedant Kumar via Phabricator via lldb-commits 
vsk accepted this revision.
vsk added a comment.
This revision is now accepted and ready to land.

Sweet! Does this 'automatically' fix the 'llvm-argdumper has issues escaping 
JSON-ified input' issue we discussed in person?


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68248/new/

https://reviews.llvm.org/D68248



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68248: [JSON] Use LLVM's library for encoding JSON

2019-09-30 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere marked an inline comment as done.
JDevlieghere added inline comments.



Comment at: 
lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp:387
   std::string expected_packet1 =
-  R"(jTraceStart:{"buffersize" : 8192,"metabuffersize" : 8192,"params" :)";
+  R"(jTraceStart:{"buffersize":8192,"metabuffersize":8192,"params":)";
   std::string expected_packet2 =

shafik wrote:
> Removing the spaces, is this just a formatting change?
Yes


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68248/new/

https://reviews.llvm.org/D68248



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68248: [JSON] Use LLVM's library for encoding JSON

2019-09-30 Thread Shafik Yaghmour via Phabricator via lldb-commits 
shafik added inline comments.



Comment at: 
lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp:387
   std::string expected_packet1 =
-  R"(jTraceStart:{"buffersize" : 8192,"metabuffersize" : 8192,"params" :)";
+  R"(jTraceStart:{"buffersize":8192,"metabuffersize":8192,"params":)";
   std::string expected_packet2 =

Removing the spaces, is this just a formatting change?


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68248/new/

https://reviews.llvm.org/D68248



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68248: [JSON] Use LLVM's library for encoding JSON

2019-09-30 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere created this revision.
JDevlieghere added reviewers: labath, mgorny, xiaobai, aprantl, clayborg.
Herald added a subscriber: abidh.
Herald added a project: LLDB.

This patch replaces the hand-rolled JSON emission with LLVM's JSON library.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D68248

Files:
  lldb/include/lldb/Core/StructuredDataImpl.h
  lldb/include/lldb/Utility/StructuredData.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
  lldb/source/Utility/StructuredData.cpp
  lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp

Index: lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
===
--- lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -384,9 +384,9 @@
 
   // Since the line is exceeding 80 characters.
   std::string expected_packet1 =
-  R"(jTraceStart:{"buffersize" : 8192,"metabuffersize" : 8192,"params" :)";
+  R"(jTraceStart:{"buffersize":8192,"metabuffersize":8192,"params":)";
   std::string expected_packet2 =
-  R"( {"psb" : 1,"tracetech" : "intel-pt"},"threadid" : 35,"type" : 1})";
+  R"({"psb":1,"tracetech":"intel-pt"},"threadid":35,"type":1})";
   HandlePacket(server, (expected_packet1 + expected_packet2), "1");
   ASSERT_TRUE(error.Success());
   ASSERT_EQ(result.get(), 1u);
@@ -409,8 +409,7 @@
 return client.SendStopTracePacket(trace_id, thread_id);
   });
 
-  const char *expected_packet =
-  R"(jTraceStop:{"threadid" : 35,"traceid" : 3})";
+  const char *expected_packet = R"(jTraceStop:{"threadid":35,"traceid":3})";
   HandlePacket(server, expected_packet, "OK");
   ASSERT_TRUE(result.get().Success());
 
@@ -435,8 +434,8 @@
   });
 
   std::string expected_packet1 =
-  R"(jTraceBufferRead:{"buffersize" : 32,"offset" : 0,"threadid" : 35,)";
-  std::string expected_packet2 = R"("traceid" : 3})";
+  R"(jTraceBufferRead:{"buffersize":32,"offset":0,"threadid":35,)";
+  std::string expected_packet2 = R"("traceid":3})";
   HandlePacket(server, expected_packet1+expected_packet2, "123456");
   ASSERT_TRUE(result.get().Success());
   ASSERT_EQ(buffer.size(), 3u);
@@ -467,8 +466,8 @@
   });
 
   std::string expected_packet1 =
-  R"(jTraceMetaRead:{"buffersize" : 32,"offset" : 0,"threadid" : 35,)";
-  std::string expected_packet2 = R"("traceid" : 3})";
+  R"(jTraceMetaRead:{"buffersize":32,"offset":0,"threadid":35,)";
+  std::string expected_packet2 = R"("traceid":3})";
   HandlePacket(server, expected_packet1+expected_packet2, "123456");
   ASSERT_TRUE(result.get().Success());
   ASSERT_EQ(buffer.size(), 3u);
@@ -497,11 +496,10 @@
   });
 
   const char *expected_packet =
-  R"(jTraceConfigRead:{"threadid" : 35,"traceid" : 3})";
+  R"(jTraceConfigRead:{"threadid":35,"traceid":3})";
   std::string response1 =
-  R"({"buffersize" : 8192,"params" : {"psb" : 1,"tracetech" : "intel-pt"})";
-  std::string response2 =
-  R"(],"metabuffersize" : 8192,"threadid" : 35,"type" : 1}])";
+  R"({"buffersize":8192,"params":{"psb":1,"tracetech":"intel-pt"})";
+  std::string response2 = R"(],"metabuffersize":8192,"threadid":35,"type":1}])";
   HandlePacket(server, expected_packet, response1+response2);
   ASSERT_TRUE(result.get().Success());
   ASSERT_EQ(options.getTraceBufferSize(), 8192u);
Index: lldb/source/Utility/StructuredData.cpp
===
--- lldb/source/Utility/StructuredData.cpp
+++ lldb/source/Utility/StructuredData.cpp
@@ -11,7 +11,6 @@
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/JSON.h"
 #include "lldb/Utility/Status.h"
-#include "lldb/Utility/Stream.h"
 #include "lldb/Utility/StreamString.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/MemoryBuffer.h"
@@ -21,6 +20,7 @@
 #include 
 
 using namespace lldb_private;
+using namespace llvm;
 
 // Functions that use a JSONParser to parse JSON into StructuredData
 static StructuredData::ObjectSP ParseJSONValue(JSONParser _parser);
@@ -181,98 +181,48 @@
 }
 
 void StructuredData::Object::DumpToStdout(bool pretty_print) const {
-  StreamString stream;
-  Dump(stream, pretty_print);
-  llvm::outs() << stream.GetString();
+  json::OStream stream(llvm::outs(), pretty_print);
+  Serialize(stream);
 }
 
-void StructuredData::Array::Dump(Stream , bool pretty_print) const {
-  bool first = true;
-  s << "[";
-  if (pretty_print) {
-s << "\n";
-s.IndentMore();
-  }
+void StructuredData::Array::Serialize(json::OStream ) const {
+  s.arrayBegin();
   for (const auto _sp : m_items) {
-if (first) {
-  first = false;
-} else {
-  s << ",";
-  if (pretty_print)
-s << "\n";
-}
-
-if (pretty_print)
-  s.Indent();
-