[PATCH] D66526: [utils] Add the llvm-locstats tool

Djordje Todorovic via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 17 23:31:08 PDT 2019


djtodoro marked an inline comment as done.
djtodoro added inline comments.


================
Comment at: utils/llvm-locstats/llvm-locstats.py:120
+  # that as an escape character.
+  cmd_stdout = cmd_stdout.replace('\\','')
+
----------------
vsk wrote:
> djtodoro wrote:
> > djtodoro wrote:
> > > vsk wrote:
> > > > I'm a bit confused by this. Mind sharing a link / description for the error?
> > > > 
> > > > Did llvm-dwarfdump produce invalid (unescaped) JSON, thus causing python's JSON parser to fail?
> > > Sure.
> > > 
> > > When try to load the JSON from `llvm-dwarfdump` output with `loads()` from json package it trows an exception on Windows platform.
> > > 
> > > `Invalid \escape: line 1 column 23 (char 23)`
> > > 
> > > The reason of the exception is special character '\' from Windows path. I think that `llvm-dwarfdump` should handle that, so this won't be needed here.
> > @vsk Do you think we should handle this on the `llvm-dwarfdump` side, since we have the JSON that breaks the specification (the JSON spec)?
> Absolutely, llvm-dwarfdump needs to produce valid json, as I'm sure there are clients that rely on that. I understand that may be more work than you signed up for :). If you don't have the cycles for that, let me know.
This will be a small fix. Thanks for your comments and reviews! :)


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

https://reviews.llvm.org/D66526





More information about the llvm-commits mailing list