cjdb accepted this revision.
cjdb added a comment.
This revision is now accepted and ready to land.
Open comments notwithstanding, I'm happy with this patch. Thank you for working
on this, it's a huge step towards getting more helpful diagnostics for C++!
@aaron.ballman, @vaibhav.y, do you folks have any further concerns? (@denik has
some commentary incoming)
================
Comment at: clang/include/clang/Frontend/SARIFDiagnostic.h:24-26
+ raw_ostream *OS;
+
+ SarifDocumentWriter *Writer;
----------------
These can go in the private section below.
================
Comment at: clang/include/clang/Frontend/SARIFDiagnosticPrinter.h:72
+
+ bool OwnsOutputStream = true;
+};
----------------
This can't ever happen, because there's no constructor that doesn't set
`OwnsOutputStream`.
================
Comment at: clang/lib/Frontend/SARIFDiagnostic.cpp:86-88
+ if (Range.isInvalid()) {
+ continue;
+ }
----------------
It seems @aaron.ballman has finally trained me :(
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
