denik accepted this revision. denik added inline comments.
================ Comment at: clang/lib/Frontend/SARIFDiagnostic.cpp:58 + + if (Loc.isValid()) + Result = addLocationToResult(Result, Loc, PLoc, Ranges, *Diag); ---------------- abrahamcd wrote: > denik wrote: > > I think we should add a test case when Loc or/and Source Manager is not set. > > If Loc is not set SarifDocumentWriter might not create the `locations` > > property which is required by the spec. > > If this is the case we need to fix it. But I'm not sure if > > emitDiagnosticMessage() is going to be called when Loc.isInvalid() ). > I believe if Loc is invalid, it goes to one of those special cases I > mentioned in this review. If it does then "if" is redundant here. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131632/new/ https://reviews.llvm.org/D131632 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
