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.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D109701/new/
https://reviews.llvm.org/D109701
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
