[PATCH] D145284: WIP [clang] adds capabilities for SARIF to be written to file

vaibhav.y added inline comments. Comment at: clang/lib/Tooling/SarifLinker.cpp:21 + +namespace llvm::sarif_linker { +[[noreturn]] void ReportErrorAndExit(llvm::errc error_code, StringRef Message) { Is this for link diagnostics reported as SARIF? Repository:

[PATCH] D146654: [clang] replaces numeric SARIF ids with heirarchical names

vaibhav.y added inline comments. Comment at: clang/lib/Frontend/SARIFDiagnostic.cpp:51-52 + Diag->getDiags()->getDiagnosticIDs()->getStableName(Diag->getID()).str(); + std::replace(StableName.begin(), StableName.end(), '_', '.'); + SarifRule Rule = SarifRule::create().setR

[PATCH] D135987: [clangBasic] Refactor StaticAnalyzer to use `clang::SarifDocumentWriter`

vaibhav.y updated this revision to Diff 476186. vaibhav.y added a comment. Rebase on upstream/main Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135987/new/ https://reviews.llvm.org/D135987 Files: clang/include/clang/Basic/SourceLocation.h cla

[PATCH] D135987: [clangBasic] Refactor StaticAnalyzer to use `clang::SarifDocumentWriter`

vaibhav.y added a comment. Great! Please use the following for patch attribution: - Name: Vaibhav Yenamandra - email address: vyenaman...@bloomberg.net Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135987/new/ https://reviews.llvm.org/D135987 ___

[PATCH] D135987: [clangBasic] Refactor StaticAnalyzer to use `clang::SarifDocumentWriter`

vaibhav.y added a comment. In D135987#3906253 , @aaron.ballman wrote: > Thank you for this refactoring, I really like the direction it's going! I've > not spotted anything concerning, but if someone can double-check conformance > to SARIF in terms of t

[PATCH] D135987: [clangBasic] Refactor StaticAnalyzer to use `clang::SarifDocumentWriter`

vaibhav.y updated this revision to Diff 475475. vaibhav.y added a comment. Update FileCheck tests to include now serialized file URIs Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135987/new/ https://reviews.llvm.org/D135987 Files: clang/include

[PATCH] D135987: [clangBasic] Refactor StaticAnalyzer to use `clang::SarifDocumentWriter`

vaibhav.y created this revision. vaibhav.y added reviewers: aaron.ballman, cjdb. Herald added subscribers: steakhal, wenlei, martong. Herald added a reviewer: NoQ. Herald added a project: All. vaibhav.y updated this revision to Diff 472133. vaibhav.y added a comment. vaibhav.y added a reviewer: dbe

[PATCH] D131632: [clang] Enable output of SARIF diagnostics

vaibhav.y added inline comments. Comment at: clang/lib/Frontend/SARIFDiagnosticPrinter.cpp:71 + // other infrastructure necessary when emitting more rich diagnostics. + if (!Info.getLocation().isValid()) { // TODO: What is this case? +// SARIFDiag->addDiagnosticWithoutLoca

[PATCH] D131632: [clang] Enable output of SARIF diagnostics

vaibhav.y added inline comments. Comment at: clang/lib/Frontend/SARIFDiagnostic.cpp:155 +break; + } +} Does this need an `llvm_unreachable` after the switch? Comment at: clang/lib/Frontend/SARIFDiagnostic.cpp:177 + // original path as

[PATCH] D131084: Add support for specifying the severity of a SARIF Result.

vaibhav.y added a comment. Thank you! I don't have merge access, will need your help for that :) For the commit details, I'd like to use: - Name: Vaibhav Yenamandra - email address: vyenaman...@bloomberg.net Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D131084: Add support for specifying the severity of a SARIF Result.

vaibhav.y updated this revision to Diff 453427. vaibhav.y edited the summary of this revision. vaibhav.y added a comment. Address review comments: - Drop include line that added `llvm/Support/ErrorHandling.h` - Use `llvm::Optional::value_or(...)` instead of bare conditional - Add `SarifResultLeve

[PATCH] D131084: Add support for specifying the severity of a SARIF Result.

vaibhav.y marked 4 inline comments as done. vaibhav.y added inline comments. Comment at: clang/include/clang/Basic/Sarif.h:157 +/// 1. https://docs.oasis-open.org/sarif/sarif/v2.1.0/os/sarif-v2.1.0-os.html#_Toc34317648";>level property +enum class SarifResultLevel { Note, Warnin

[PATCH] D131632: [clang] Enable output of SARIF diagnostics

vaibhav.y added inline comments. Comment at: clang/lib/Frontend/SARIFDiagnostic.cpp:75 +emitFilename(FE->getName(), Loc.getManager()); +// FIXME: No current way to add file-only location to SARIF object + } abrahamcd wrote: > @vaibhav.y ,

[PATCH] D131084: Add support for specifying the severity of a SARIF Result.

vaibhav.y marked an inline comment as not done. vaibhav.y added inline comments. Comment at: clang/include/clang/Basic/Sarif.h:157 +/// 1. https://docs.oasis-open.org/sarif/sarif/v2.1.0/os/sarif-v2.1.0-os.html#_Toc34317648";>level property +enum class SarifResultLevel { Note, Wa

[PATCH] D131084: Add support for specifying the severity of a SARIF Result.

vaibhav.y updated this revision to Diff 451635. vaibhav.y added a comment. Run git-clang-format Rebase on upstream Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131084/new/ https://reviews.llvm.org/D131084 Files: clang/include/clang/Basic/Sarif

[PATCH] D131084: Add support for specifying the severity of a SARIF Result.

vaibhav.y updated this revision to Diff 451633. vaibhav.y added a comment. Autofix clang-tidy readability warnings Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131084/new/ https://reviews.llvm.org/D131084 Files: clang/include/clang/Basic/Sarif.

[PATCH] D131084: Add support for specifying the severity of a SARIF Result.

vaibhav.y added inline comments. Comment at: clang/include/clang/Basic/Sarif.h:256 std::string HelpURI; + SarifReportingConfiguration DefaultConfiguration; Is this form preferred for expressing a statically known default value? Repository: rG LLVM Githu

[PATCH] D131084: Add support for specifying the severity of a SARIF Result.

vaibhav.y updated this revision to Diff 451585. vaibhav.y added a comment. Update diff: - Create class `SarifReportingConfiguration`: This should allow rules to specify the `defaultConfiguration` property Add tests for: - Creating rules with custom reporting configuration - Death test for inva

[PATCH] D131084: Add support for specifying the severity of a SARIF Result.

vaibhav.y added a comment. > A part of my endgame is to see notes be incorporated into their parents, but > that's a long way off methinks. Regarding this, the current best approach the spec provides is using the "locationRelationShip"

[PATCH] D131084: Add support for specifying the severity of a SARIF Result.

vaibhav.y added a comment. > Hmm, we can probably use "informational" for notes, warnings, and remarks, > but I'm kinda partial to proposing the latter two upstream. Interesting, I agree that remarks could fit under "informational". As for notes: As I understand it they are always child diagno

[PATCH] D131084: Add support for specifying the severity of a SARIF Result.

vaibhav.y created this revision. vaibhav.y added reviewers: aaron.ballman, cjdb, denik, abrahamcd, dbeer1. Herald added a project: All. vaibhav.y edited the summary of this revision. vaibhav.y edited the summary of this revision. vaibhav.y updated this revision to Diff 449713. vaibhav.y added a com

[PATCH] D129886: [clang] Add -fdiagnostics-format=sarif option for future SARIF output

vaibhav.y added a comment. Please address the whitespace changes in `clang/lib/Driver/ToolChains/Clang.cpp`. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:80-81 Args.getLastArg(options::OPT_dynamic, options::OPT_mdynamic_no_pic)) - D.Diag(diag::err_drv_a

[PATCH] D129886: [clang] Add -fdiagnostics-format=sarif option for future SARIF output

vaibhav.y added a comment. In D129886#3660299 , @abrahamcd wrote: > In D129886#3656221 , @vaibhav.y > wrote: > >> Might be worth hiding it from `--help`, despite the instability warning. > > I already couldn't fi

[PATCH] D129886: [clang] Add -fdiagnostics-format=sarif for SARIF diagnostics

vaibhav.y added a comment. Might be worth hiding it from `--help`, despite the instability warning. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129886/new/ https://reviews.llvm.org/D129886 ___ cfe-comm

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y updated this revision to Diff 444960. vaibhav.y added a comment. Undo test case renames Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109701/new/ https://reviews.llvm.org/D109701 Files: clang/include/clang/Basic/Sarif.h clang/lib/Bas

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y updated this revision to Diff 444764. vaibhav.y added a comment. Gate death tests on `NDEBUG` and available of `GTEST_HAS_DEATH_TEST` This should fix recent pre-merge failures Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109701/new/ htt

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y added a comment. @aaron.ballman The culprit turned out to be the difference in release flags on the build server vs my environment. I had unfortunately run the configuration command once in `Debug` mode, and hadn't re-configured. Not a bright moment :) The `ASSERT_DEATH` tests that I

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y added a comment. In D109701#3646856 , @abrahamcd wrote: > Hi! I'm interning with @cjdb and @denik this summer and I was working on > adding a `-fdiagnostics-format=sarif` option to start off my project, but I > just found that a previous aband

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y added a comment. In D109701#3642748 , @aaron.ballman wrote: > Unfortunately, it still seems to be causing failures (I had to revert again): > > https://lab.llvm.org/buildbot/#/builders/91/builds/11840 > > It looks to be the same failure as befo

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y added a comment. >> I have suspicions that it was the `SarifResult &&` type in the test so I >> changed it to a `const SarifResult &`. >> >> I've tried running it on a RHEL 7, Darwin on my end. > > If you think you've got it fixed, I think the best we can do is to re-commit > and watch

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y updated this revision to Diff 443654. vaibhav.y added a comment. Discard most likely culprit in test code causing unexpected crash. @aaron.ballman I was unable to reproduce the issue on my end using the buildbot instructions and ASAN, UBSAN. Is it possible for you to trigger a pre-merg

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y added a comment. In D109701#3622983 , @aaron.ballman wrote: > I had to roll it back because of failures with test bots: > > https://lab.llvm.org/buildbot/#/builders/91/builds/11328 > > So this was reverted in b46ad1b5be694feefabd4c6cd112cbbd04a

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y updated this revision to Diff 441130. vaibhav.y added a comment. Assert that `CharSourceRange`s passed to `Threadflow`, `SarifResult` are character Ranges. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109701/new/ https://reviews.llvm.org

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y added inline comments. Comment at: clang/include/clang/Basic/Sarif.h:167 + + ThreadFlow setRange(const CharSourceRange &ItemRange) { +Range = ItemRange; aaron.ballman wrote: > Should we assert this source range is not a token range? I don't have a

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y updated this revision to Diff 440374. vaibhav.y added a comment. Discard include: `clang/Lex/Lexer.h` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109701/new/ https://reviews.llvm.org/D109701 Files: clang/include/clang/Basic/Sarif.h

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y updated this revision to Diff 440371. vaibhav.y added a comment. Use `CharSourceRange` instead of FullSourceRange so we no longer need to compute the length of the last token. This should already be encoded in the end location of the `CharSourceRange` by the caller TODO(@vaibhav.y): Onc

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y added inline comments. Comment at: clang/lib/Basic/Sarif.cpp:161 +Region["endColumn"] = adjustColumnPos( +R.getEnd(), Lexer::MeasureTokenLength(R.getEnd().getLocWithOffset(0), + R.getEnd().getManager(), LO));

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y marked an inline comment as not done. vaibhav.y added a comment. Comment isn't done. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109701/new/ https://reviews.llvm.org/D109701 ___ cfe-commits m

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y added inline comments. Comment at: clang/lib/Basic/Sarif.cpp:161 +Region["endColumn"] = adjustColumnPos( +R.getEnd(), Lexer::MeasureTokenLength(R.getEnd().getLocWithOffset(0), + R.getEnd().getManager(), LO));

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y updated this revision to Diff 440279. vaibhav.y added a comment. Discard unused includes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109701/new/ https://reviews.llvm.org/D109701 Files: clang/include/clang/Basic/Sarif.h clang/includ

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y added inline comments. Comment at: clang/lib/Basic/Sarif.cpp:161 +Region["endColumn"] = adjustColumnPos( +R.getEnd(), Lexer::MeasureTokenLength(R.getEnd().getLocWithOffset(0), + R.getEnd().getManager(), LO));

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y updated this revision to Diff 440270. vaibhav.y added a comment. Factor dependency on `Lexer::MeasureTokenLength` into externally provided functor Introduces a type: `TokenLengthMetric` which measures the length of a token starting at the given SLoc Repository: rG LLVM Github Monor

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y added inline comments. Comment at: clang/lib/Basic/Sarif.cpp:161 +Region["endColumn"] = adjustColumnPos( +R.getEnd(), Lexer::MeasureTokenLength(R.getEnd().getLocWithOffset(0), + R.getEnd().getManager(), LO));

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y added a comment. >> I will update the commit messages, but I cannot commit to the github repo. > > Ah, thank you for letting me know. I can land the changes on your behalf. > What name and email address would you like me to use for patch attribution? > (I probably won't land it until t

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y updated this revision to Diff 439507. vaibhav.y added a comment. Reword commit messages - Prefix `[tag]` specifying which components are affected - Append link to D109701 in commit message per commit message guidelines Repository: rG LLVM Github M

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y added a comment. Thanks for your patience with the review as well! Just noticed that I need to add a link to revision in the commit messages as well: (https://www.llvm.org/docs/Phabricator.html#committing-a-change) I will update the commit messages, but I cannot commit to the github r

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y updated this revision to Diff 439153. vaibhav.y added a comment. Fix formatting in Sarif.h Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109701/new/ https://reviews.llvm.org/D109701 Files: clang/include/clang/Basic/Sarif.h clang/incl

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y updated this revision to Diff 439096. vaibhav.y added a comment. Discard top-level const specifier where target isn't pointee/ref Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109701/new/ https://reviews.llvm.org/D109701 Files: clang/i

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y marked an inline comment as done. vaibhav.y added a comment. Thanks, will push changes to address the comments soon. As I understood from our discussion the work @cjdb has planned would create a new `DiagnosticsConsumer`, it can be started in parallel but would need the changes in D10

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y updated this revision to Diff 436110. vaibhav.y marked 2 inline comments as done. vaibhav.y added a comment. Fix bug detected from ASAN run Passes `-fsanitize=address` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109701/new/ https://rev

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y updated this revision to Diff 436030. vaibhav.y added a comment. Rebase on main branch from upstream git Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109701/new/ https://reviews.llvm.org/D109701 Files: clang/include/clang/Basic/Sarif.

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y added a comment. https://discourse.llvm.org/t/rfc-improving-clang-s-diagnostics/62584/8 There is an ongoing RFC similar to the work here, worth noting. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109701/new/ https://reviews.llvm.org/D1

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y added a comment. @aaron.ballman Would it be possible that I add `::validate` through a follow-up PR? I'm currently checking the JSON output from the writer using Microsoft's online validator , and it is passing. Though it tends to compla

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y updated this revision to Diff 434802. vaibhav.y added a comment. Update tests to check serialization as well - SARIF text generated is validated externally against [Microsoft's online validator][1] [1]: https://sarifweb.azurewebsites.net/Validation Repository: rG LLVM Github Monor

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y added a comment. Hi, Apologies for the long delay! I was on a short break to focus to other projects. I have some changes in mind such as: - Creating the `SarifLog` object to represent the top-level document. Currently we store this as a JSON object which ends up rather mucky. Having

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y updated this revision to Diff 414743. vaibhav.y marked 15 inline comments as done. vaibhav.y added a comment. Herald added a project: All. Fixup comments Mark clang::FullSourceRange as returning const & to its locs rebase on upstream main Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y added a comment. ping: Requesting review Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109701/new/ https://reviews.llvm.org/D109701 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y added a comment. ping: This is ready for review now. Thanks for your patience with the review as well! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109701/new/ https://reviews.llvm.org/D109701 __

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y marked 11 inline comments as done. vaibhav.y added a comment. Mark completed comments as done Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109701/new/ https://reviews.llvm.org/D109701 ___ cfe-

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y updated this revision to Diff 390448. vaibhav.y added a comment. Rename enum members Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109701/new/ https://reviews.llvm.org/D109701 Files: clang/include/clang/Basic/Sarif.h clang/include/cl

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y added inline comments. Comment at: clang/include/clang/Basic/Sarif.h:80-82 + static SarifArtifactLocation create(StringRef URI) { +return SarifArtifactLocation{URI}; + } aaron.ballman wrote: > One thing that's worth calling out is that `StringRef`

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y updated this revision to Diff 390396. vaibhav.y marked 6 inline comments as done. vaibhav.y added a comment. Rebase on upstream/main: - [clangBasic] Format code - [clangBasic] Mark all constructors taking single values `explicit` - [clangBasic] Convert `StringRef` to `std::string` field

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y added a comment. Ping. `clang-format` is the only check failing as of now (attempting to reformat the `` links in doc comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109701/new/ https://reviews.llvm.org/D109701

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y updated this revision to Diff 385813. vaibhav.y added a comment. Format code using patch from buildkite Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109701/new/ https://reviews.llvm.org/D109701 Files: clang/include/clang/Basic/Sarif.h

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y updated this revision to Diff 385636. vaibhav.y added a comment. Format code Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109701/new/ https://reviews.llvm.org/D109701 Files: clang/include/clang/Basic/Sarif.h clang/include/clang/Basi

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y updated this revision to Diff 385634. vaibhav.y added a comment. Add `Closed` to signal if document is writable Introduce a flag `Closed` to `clang::SarifDocumentWriter` that tracks if the underlying sarif document is accepting results to record. i.e. it is in the middle of an acti

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y updated this revision to Diff 385609. vaibhav.y added a comment. Summary of changes: - [clangBasic] Fix SIGSEGV in Sarif builders - [clangBasic] Fixup documentation typo, add explicit type Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y added inline comments. Comment at: clang/include/clang/Basic/Sarif.h:95 +// +/// Since every in clang artifact MUST have a location (there being no nested +/// artifacts), the creation method \ref SarifArtifact::create requires a aaron.ballman wrote: >

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y updated this revision to Diff 375734. vaibhav.y marked 34 inline comments as done. vaibhav.y added a comment. Address comments from upstream review: - Mark anonymous functions static, improve documentation - Coding style fixes - Use Ref types as they are intended (as non-owning lightwei

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y updated this revision to Diff 372548. vaibhav.y added a comment. Format clang/lib/Basic/Sarif.cpp Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109701/new/ https://reviews.llvm.org/D109701 Files: clang/include/clang/Basic/Sarif.h cla

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y updated this revision to Diff 372543. vaibhav.y added a comment. Fixup name of class defined in documentation Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109701/new/ https://reviews.llvm.org/D109701 Files: clang/include/clang/Basic/S

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y updated this revision to Diff 372542. vaibhav.y added a comment. Address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109701/new/ https://reviews.llvm.org/D109701 Files: clang/include/clang/Basic/Sarif.h clang/includ

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y added inline comments. Comment at: clang/lib/Basic/Sarif.cpp:1 +#include "clang/Basic/Sarif.h" +#include "clang/Basic/LangOptions.h" lattner wrote: > THis nees the standard header boilerplate per the coding standards doc Ack, didn't grok the "all source

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y added a comment. > Btw, is the intent for this functionality to replace what's in > SarifDiagnostics.cpp > (https://github.com/llvm/llvm-project/blob/main/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp) > fairly immediately? Or are we going to carry both implementations? This i

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y added inline comments. Comment at: clang/include/clang/Basic/Sarif.h:69 +/// Reference: +/// 1. https://docs.oasis-open.org/sarif/sarif/v2.1.0/os/sarif-v2.1.0-os.html#_Toc34317427";>artifactLocation object +/// 2. \ref SarifArtifact I'm not sure how to

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y updated this revision to Diff 372289. vaibhav.y added a comment. [clangBasic] Fixup header guard Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109701/new/ https://reviews.llvm.org/D109701 Files: clang/include/clang/Basic/Sarif.h clan

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

vaibhav.y created this revision. vaibhav.y added a reviewer: lattner. Herald added subscribers: dexonsmith, wenlei, mgorny. vaibhav.y edited the summary of this revision. vaibhav.y added a project: clang. vaibhav.y added subscribers: aaron.ballman, lebedev.ri. vaibhav.y added a comment. vaibhav.y e