[PATCH] D102355: Add support for DWARF embedded source to llvm-symbolizer

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-symbolizer/source.ll:1
+;; This test checks printout of the DWARF embedded source.
+
----------------
I think "output" is a more common term to use and easier to understand.


================
Comment at: llvm/test/tools/llvm-symbolizer/source.ll:7
+
+;; Check the source context plain printout.
+; RUN: llvm-symbolizer --print-source-context-lines=3 "CODE %t.o 0" | \
----------------
No need to repeat "source context" in this text - it's implied by the top-level comment.


================
Comment at: llvm/test/tools/llvm-symbolizer/source.ll:8
+;; Check the source context plain printout.
+; RUN: llvm-symbolizer --print-source-context-lines=3 "CODE %t.o 0" | \
+; RUN:   FileCheck %s --strict-whitespace --match-full-lines --implicit-check-not={{.}}
----------------
This is simpler and more traditional for the input arguments. Also CODE is the default and not needed.


================
Comment at: llvm/test/tools/llvm-symbolizer/source.ll:16
+
+;; Check the source context plain printout for GNU output style.
+; RUN: llvm-symbolizer --print-source-context-lines=3 "CODE %t.o 0" --output-style=GNU | \
----------------



================
Comment at: llvm/test/tools/llvm-symbolizer/source.ll:17
+;; Check the source context plain printout for GNU output style.
+; RUN: llvm-symbolizer --print-source-context-lines=3 "CODE %t.o 0" --output-style=GNU | \
+; RUN:   FileCheck %s --check-prefix=GNU --strict-whitespace --match-full-lines --implicit-check-not={{.}}
----------------



================
Comment at: llvm/test/tools/llvm-symbolizer/source.ll:19-23
+;      GNU:foo
+; GNU-NEXT:/source.c:3
+; GNU-NEXT:2  : // Line 2
+; GNU-NEXT:3 >: void foo() {}
+; GNU-NEXT:4  : // Line 4
----------------
There's only one small difference between this and the LLVM style, if I'm not mistaken. You could avoid duplciation by doing something like:
```
; RUN: ... | FileCheck ... --check-prefixes=COMMON,LLVM;
; RUN: ... | FileCheck ... --check-prefixes=COMMON,GNU

;      COMMON:foo
;   LLVM-NEXT:/source.c:3:13
;    GNU-NEXT:/source.c:3
; COMMON-NEXT:2  : // Line 2
; COMMON-NEXT:3 >: void foo() {}
; COMMON-NEXT:4  : // Line 4
```


================
Comment at: llvm/test/tools/llvm-symbolizer/source.ll:25
+
+;; Check the source context printout to JSON.
+; RUN: llvm-symbolizer --print-source-context-lines=3 "CODE %t.o 0" --output-style=JSON | \
----------------



================
Comment at: llvm/test/tools/llvm-symbolizer/source.ll:26
+;; Check the source context printout to JSON.
+; RUN: llvm-symbolizer --print-source-context-lines=3 "CODE %t.o 0" --output-style=JSON | \
+; RUN:   FileCheck %s --check-prefix=JSON --strict-whitespace --match-full-lines --implicit-check-not={{.}}
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102355