This is an archive of the discontinued LLVM Phabricator instance.

[JSON] Use LLVM's library for encoding JSON
ClosedPublic

Authored by JDevlieghere on Sep 30 2019, 3:49 PM.

Diff Detail

Event Timeline

JDevlieghere created this revision.Sep 30 2019, 3:49 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 30 2019, 3:49 PM
Herald added a subscriber: abidh. · View Herald Transcript
shafik added a subscriber: shafik.Sep 30 2019, 4:18 PM
shafik added inline comments.
lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
387

Removing the spaces, is this just a formatting change?

JDevlieghere marked an inline comment as done.Sep 30 2019, 4:26 PM
JDevlieghere added inline comments.
lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
387

Yes

vsk accepted this revision.Sep 30 2019, 5:20 PM

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

This revision is now accepted and ready to land.Sep 30 2019, 5:20 PM
labath accepted this revision.Oct 1 2019, 12:10 AM

Yay.

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

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..

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.

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.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptOct 1 2019, 10:39 AM