[PATCH] D62952: [analyzer][tests] Use normalize_sarif in place of diff_sarif

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D62952#1548580 <https://reviews.llvm.org/D62952#1548580>, @hubert.reinterpretcast wrote:

> In D62952#1548377 <https://reviews.llvm.org/D62952#1548377>, @aaron.ballman wrote:
>
> > In general, that seems reasonable, but I would prefer to take care of more of the work in lit.local.cfg than have to deal with that atrocious RUN line in every test case. Is there a way to retain a similarly succinct solution as diff_sarif?
>
>
> There'd be no atrocious RUN line if we went with modifying the expected files beforehand and having the tool output a newline.
>
> The unchanged:
>
>   // RUN: %clang_analyze_cc1 -analyzer-checker=alpha.security.taint,debug.TaintTest %s -verify -analyzer-output=sarif -o - | %diff_sarif %S/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif -
>
>
> becomes:
>
>   // RUN: %clang_analyze_cc1 -analyzer-checker=alpha.security.taint,debug.TaintTest %s -verify -analyzer-output=sarif -o - | %normalize_sarif | diff -U1 -b %S/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif -
>
>
> As in, `%diff_sarif` gets replaced with `%normalize_sarif | diff -U1 -b` and that's it.


But is there a reason to not keep `%diff_sarif` and define it in terms of `%normalize_sarif | diff -U1 -b` within lit.local.cfg? I guess I don't see the benefit to exposing the call to diff (I don't anticipate anyone needing to change the options passed to diff).


Repository:
  rC Clang

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

https://reviews.llvm.org/D62952