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

Vaibhav Yenamandra via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 29 11:47:58 PDT 2022

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 strong opinion here (since these are validated downstream in `createPhysicalLocation`) but it makes sense to be defensive & assert early.

I'll preserve the downstream one as well in case a new type gets added that feeds data into `createPhysicalLocation` as well.

Comment at: clang/include/clang/Basic/Sarif.h:280
+  SarifResult setLocations(llvm::ArrayRef<CharSourceRange> DiagLocs) {
+    Locations.assign(DiagLocs.begin(), DiagLocs.end());
+    return *this;
aaron.ballman wrote:
> In an asserts build, should we additionally have a loop to assert that each location is a char range rather than a token range?
Will do, I had the asserts in `createPhysicalLocation` initially. Adding them at the site of creation makes sense as well.

  rG LLVM Github Monorepo



More information about the cfe-commits mailing list