denik added a comment. Thanks Abraham!
Please mention in the summary that some part of SARIF Diag implementation was copied from Text Diag and further refactoring is needed. Also, like we discussed: - drop the unittest; - replace strict match of json output in FileCheck with partial matches of the fields that we are interested in (if possible); - rebase and incorporate setDefaultConfiguration from D131084 <https://reviews.llvm.org/D131084>. - on the `logicalLocations` I have to take a look if we can add it in this patch or defer it and add a warning that this diagnostic is currently unsupported. ================ Comment at: clang/include/clang/Frontend/SARIFDiagnosticPrinter.h:40 + + SarifDocumentWriter *Writer = nullptr; + ---------------- std::unique_ptr<SarifDocumentWriter> Writer; ================ Comment at: clang/lib/Frontend/SARIFDiagnosticPrinter.cpp:48 + Writer->endRun(); + llvm::json::Value value(Writer->createDocument()); + OS << "\n" << value << "\n\n"; ---------------- Value(Writer->createDocument()); 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
