This revision was automatically updated to reflect the committed changes.
Closed by commit rL311483: [lldb] Correctly escape newlines and backslashes in
the JSON serializer (authored by kuba.brecka).
Changed prior to commit:
https://reviews.llvm.org/D34322?vs=102950=112206#toc
Repository:
jasonmolenda accepted this revision.
jasonmolenda added a comment.
This revision is now accepted and ready to land.
LGTM.
https://reviews.llvm.org/D34322
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
joerg added a comment.
Not parsers, encoders. Note that the escaping is not correct for control
characters other than \n.
https://reviews.llvm.org/D34322
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
beanz added a comment.
I get your perspective, but holding up this relatively small patch that fixes a
bug in existing code on an architectural disagreement seems like excessive bike
shedding. If we assume that JSON is required for the use case would you have
Kuba write a full JSON parser in
joerg added a comment.
I don't disagree with you, but please see the referenced review for further
details.
I do not want the amount of adhoc JSON encoders to grow further. The YAML
encoder works fine for most of the things, but it is easier to review calls to
it than it is to find other
beanz added a comment.
In https://reviews.llvm.org/D34322#783532, @joerg wrote:
> My suggestion would be to just use the YAML writer for now and leave a
> comment to make it clear that this is really JSON only.
Our YAML parser can parse JSON because YAML is a superset of JSON, but I don't
kubamracek added a comment.
I see. The approach to escape as little as possible can be valid, I guess.
In any case, this patch seems to do exactly this. We must escape backslashes,
otherwise we’ll produce invalid output: imagine a backslash at the end of the
string. Same about newlines:
joerg added a comment.
Please see the discussion in https://reviews.llvm.org/D31992. This patch seems
to go in the wrong direction.
https://reviews.llvm.org/D34322
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
kubamracek created this revision.
kubamracek added a project: LLDB.
Herald added a subscriber: mgorny.
JSON serializer fails to escape newlines and backslashes. Let's fix that.
https://reviews.llvm.org/D34322
Files:
lldb.xcodeproj/project.pbxproj
source/Utility/JSON.cpp
