[PATCH] D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`

cjdb added a comment. In D145201#4216567 , @aaron.ballman wrote: > In D145201#4214395 , @cjdb wrote: > >> swaps commits to see if that fixes CI (part 1) > > Oh, if this is just stacked patches confusing our preco

[PATCH] D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a tiny variable naming nit I missed before. Comment at: clang/lib/Basic/Sarif.cpp:314-317 + llvm::sort(*Artifacts, [](const json::Value &x, con

[PATCH] D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`

aaron.ballman added a comment. In D145201#4214395 , @cjdb wrote: > swaps commits to see if that fixes CI (part 1) Oh, if this is just stacked patches confusing our precommit CI, then it's okay. I hadn't realized this was a stacked patch. Repository:

[PATCH] D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`

cjdb updated this revision to Diff 507511. cjdb edited the summary of this revision. cjdb added a comment. And on this CL's side too Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145201/new/ https://reviews.llvm.org/D145201 Files: clang/include/

[PATCH] D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`

cjdb updated this revision to Diff 507497. cjdb edited the summary of this revision. cjdb added a comment. swaps commits to see if that fixes CI (part 1) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145201/new/ https://reviews.llvm.org/D145201 Fi

[PATCH] D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`

cjdb updated this revision to Diff 507137. cjdb added a comment. fixes breakage and adds FIXME Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145201/new/ https://reviews.llvm.org/D145201 Files: clang/include/clang/Frontend/SARIFDiagnostic.h cla

[PATCH] D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`

aaron.ballman added a comment. (Note, precommit CI on Windows still shows a valid failure.) Comment at: clang/lib/Frontend/SARIFDiagnostic.cpp:214 void SARIFDiagnostic::emitIncludeLocation(FullSourceLoc Loc, PresumedLoc PLoc) { - assert(false && "Not implemented in SARIF mod

[PATCH] D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`

cjdb added inline comments. Comment at: clang/lib/Frontend/SARIFDiagnostic.cpp:214 void SARIFDiagnostic::emitIncludeLocation(FullSourceLoc Loc, PresumedLoc PLoc) { - assert(false && "Not implemented in SARIF mode"); + SarifRule Rule = SarifRule::create().setRuleId(std::to_str

[PATCH] D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`

aaron.ballman added inline comments. Comment at: clang/lib/Basic/Sarif.cpp:314-317 + llvm::sort(*Artifacts, [](const json::Value &x, const json::Value &y) { +return x.getAsObject()->getNumber("index") < + y.getAsObject()->getNumber("index"); + });

[PATCH] D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`

cjdb updated this revision to Diff 502190. cjdb added a comment. fixes string goof Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145201/new/ https://reviews.llvm.org/D145201 Files: clang/include/clang/Frontend/SARIFDiagnostic.h clang/lib/Basic

[PATCH] D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`

cjdb added a comment. That test is kinda problematic because it seems that the artifacts aren't ordered. I think we should change this from a Comment at: clang/lib/Basic/Sarif.cpp:314-317 + llvm::sort(*Artifacts, [](const json::Value &x, const json::Value &y) { +return x.

[PATCH] D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`

cjdb updated this revision to Diff 502186. cjdb added a comment. Herald added a subscriber: mgrang. sorts artifacts so that they're output in index order Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145201/new/ https://reviews.llvm.org/D145201 Fi

[PATCH] D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`

aaron.ballman added a comment. It looks like there's a valid precommit CI failure that needs to be addressed: Failed Tests (1): Clang :: Frontend/sarif-diagnostics.cpp Comment at: clang/lib/Frontend/SARIFDiagnostic.cpp:214 void SARIFDiagnostic::em

[PATCH] D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`

cjdb updated this revision to Diff 502021. cjdb added a comment. fixes something that wasn't supposed to be changed in the rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145201/new/ https://reviews.llvm.org/D145201 Files: clang/include/cla

[PATCH] D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`

cjdb updated this revision to Diff 502020. cjdb added a comment. actually commits the files Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145201/new/ https://reviews.llvm.org/D145201 Files: clang/include/clang/Frontend/SARIFDiagnostic.h clang/

[PATCH] D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`

cjdb created this revision. cjdb added reviewers: aaron.ballman, shafik, erichkeane. Herald added a project: All. cjdb requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Including headers used to fire an assertion; now they report a diagnostic