[PATCH] D96883: Add support for JSON output style to llvm-symbolizer

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-symbolizer/output-style-json-frame.ll:1
+; This test checks JSON output for FRAME.
+
----------------
Does this need a `REQUIRES: aarch64-registered-target` or equivalent?


================
Comment at: llvm/test/tools/llvm-symbolizer/output-style-json-frame.ll:20
+
+; Resolve valid address.
+; RUN: llvm-symbolizer "FRAME %t.o 0" --output-style=JSON | \
----------------
Perhaps highlight that you are testing both 0 and non-zero frame offsets with a comment.


================
Comment at: llvm/test/tools/llvm-symbolizer/output-style-json-frame.ll:25
+
+target triple="aarch64--"
+
----------------
This code could probably do with some comments highlighting what the key elements are, so that future changes don't lose them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96883