Page MenuHomePhabricator

[llvm-dwarfdump] Adjust Windows path to be acceptable by JSON
ClosedPublic

Authored by djtodoro on Sep 18 2019, 2:28 AM.

Details

Summary

Backslash is a special character according to JSON specification, so we should avoid that when printing a file path with the --statistics option.

Diff Detail

Repository
rL LLVM

Event Timeline

vsk added inline comments.Sep 18 2019, 10:28 AM
tools/llvm-dwarfdump/Statistics.cpp
552 ↗(On Diff #220633)

I think '\' needs to be converted to '\\'. Consider using llvm's json::Value(Filename), which should quote things correctly? I believe that can be passed to ostream directly, so you could templat-ize printDatum (getting rid of one of its duplicate definitions) and just pass the Filename json::Value in.

djtodoro marked an inline comment as done.Sep 19 2019, 6:11 AM
djtodoro added inline comments.
tools/llvm-dwarfdump/Statistics.cpp
552 ↗(On Diff #220633)

Hmm... I didn't find any spec that says what is the desirable char in that case, but you are right. We can just pass this as a json::Value and it should know how to print that (it actually converts it into '\\'). Thanks! :)

djtodoro updated this revision to Diff 220852.Sep 19 2019, 6:14 AM

-Use JSON::Value to get the right json when printing the stats

vsk accepted this revision.Thu, Sep 19, 11:32 AM

lgtm, I believe the original llvm-locstats can now land unmodified, which should provide test coverage.

This revision is now accepted and ready to land.Thu, Sep 19, 11:32 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptFri, Sep 20, 2:25 AM