cjdb updated this revision to Diff 502308.
cjdb added a comment.
tidies up some stuff that I overlooked
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145284/new/
https://reviews.llvm.org/D145284
Files:
clang/include/clang/Basic/DiagnosticOptions.def
clang/include/clang/Basic/DiagnosticOptions.h
clang/include/clang/Driver/Options.td
clang/include/clang/Frontend/CompilerInstance.h
clang/include/clang/Frontend/SARIFDiagnosticPrinter.h
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInstance.cpp
clang/lib/Frontend/FrontendAction.cpp
clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
clang/lib/Frontend/TextDiagnostic.cpp
clang/lib/Tooling/Tooling.cpp
clang/test/Driver/fdiagnostics-format-sarif.cpp
clang/tools/driver/cc1_main.cpp
Index: clang/tools/driver/cc1_main.cpp
===================================================================
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -230,7 +230,12 @@
CompilerInvocation::GetResourcesPath(Argv0, MainAddr);
// Create the actual diagnostics engine.
- Clang->createDiagnostics();
+ auto diagnostics_format_option =
+ llvm::find(Argv, StringRef("-fdiagnostics-format"));
+ bool IsSarifFile =
+ std::distance(diagnostics_format_option, Argv.end()) > 1 &&
+ *std::next(diagnostics_format_option) == StringRef("sarif-file");
+ Clang->createDiagnostics(nullptr, true, IsSarifFile);
if (!Clang->hasDiagnostics())
return 1;
Index: clang/test/Driver/fdiagnostics-format-sarif.cpp
===================================================================
--- clang/test/Driver/fdiagnostics-format-sarif.cpp
+++ clang/test/Driver/fdiagnostics-format-sarif.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang -fsyntax-only -fdiagnostics-format=sarif %s -### 2>&1 | FileCheck %s --check-prefix=WARN
+// RUN: %clang -fsyntax-only -fdiagnostics-format=sarif-stderr %s -### 2>&1 | FileCheck %s --check-prefix=WARN
// WARN: warning: diagnostic formatting in SARIF mode is currently unstable [-Wsarif-format-unstable]
-// RUN: %clang -fsyntax-only -fdiagnostics-format=SARIF %s -### 2>&1 | FileCheck %s --check-prefix=WARN2
+// RUN: %clang -fsyntax-only -fdiagnostics-format=sarif-file %s -### 2>&1 | FileCheck %s --check-prefix=WARN2
// WARN2: warning: diagnostic formatting in SARIF mode is currently unstable [-Wsarif-format-unstable]
Index: clang/lib/Tooling/Tooling.cpp
===================================================================
--- clang/lib/Tooling/Tooling.cpp
+++ clang/lib/Tooling/Tooling.cpp
@@ -425,7 +425,9 @@
std::unique_ptr<FrontendAction> ScopedToolAction(create());
// Create the compiler's actual diagnostics engine.
- Compiler.createDiagnostics(DiagConsumer, /*ShouldOwnClient=*/false);
+ Compiler.createDiagnostics(DiagConsumer, /*ShouldOwnClient=*/false,
+ Invocation->DiagnosticOpts->getFormat() ==
+ DiagnosticOptions::SARIFFile);
if (!Compiler.hasDiagnostics())
return false;
@@ -684,7 +686,7 @@
if (!Invocation.run())
return nullptr;
-
+
assert(ASTs.size() == 1);
return std::move(ASTs[0]);
}
Index: clang/lib/Frontend/TextDiagnostic.cpp
===================================================================
--- clang/lib/Frontend/TextDiagnostic.cpp
+++ clang/lib/Frontend/TextDiagnostic.cpp
@@ -815,7 +815,8 @@
emitFilename(PLoc.getFilename(), Loc.getManager());
switch (DiagOpts->getFormat()) {
- case DiagnosticOptions::SARIF:
+ case DiagnosticOptions::SARIFStderr:
+ case DiagnosticOptions::SARIFFile:
case DiagnosticOptions::Clang:
if (DiagOpts->ShowLine)
OS << ':' << LineNo;
@@ -838,7 +839,8 @@
OS << ColNo;
}
switch (DiagOpts->getFormat()) {
- case DiagnosticOptions::SARIF:
+ case DiagnosticOptions::SARIFStderr:
+ case DiagnosticOptions::SARIFFile:
case DiagnosticOptions::Clang:
case DiagnosticOptions::Vi: OS << ':'; break;
case DiagnosticOptions::MSVC:
Index: clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
===================================================================
--- clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
+++ clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
@@ -22,19 +22,37 @@
#include "llvm/Support/JSON.h"
#include "llvm/Support/raw_ostream.h"
#include <algorithm>
+#include <memory>
namespace clang {
SARIFDiagnosticPrinter::SARIFDiagnosticPrinter(raw_ostream &OS,
DiagnosticOptions *Diags)
- : OS(OS), DiagOpts(Diags) {}
+ : OS(&OS), DiagOpts(Diags) {}
+
+static std::unique_ptr<llvm::raw_ostream>
+OpenDiagnosticFile(StringRef TargetPath, std::error_code &EC) {
+ assert(!TargetPath.empty());
+ return std::make_unique<llvm::raw_fd_ostream>(
+ std::string(TargetPath) + ".sarif", EC);
+}
+
+SARIFDiagnosticPrinter::SARIFDiagnosticPrinter(StringRef TargetPath,
+ DiagnosticOptions *Diags)
+ : EC(std::error_code()), OS(OpenDiagnosticFile(TargetPath, *EC)),
+ DiagOpts(Diags) {
+ if (*EC) {
+ assert(false and "not implemented yet: not even sure what to do");
+ }
+}
void SARIFDiagnosticPrinter::BeginSourceFile(const LangOptions &LO,
const Preprocessor *PP) {
// Build the SARIFDiagnostic utility.
assert(hasSarifWriter() && "Writer not set!");
assert(!SARIFDiag && "SARIFDiagnostic already set.");
- SARIFDiag = std::make_unique<SARIFDiagnostic>(OS, LO, &*DiagOpts, &*Writer);
+ SARIFDiag =
+ std::make_unique<SARIFDiagnostic>(getOS(), LO, &*DiagOpts, &*Writer);
// Initialize the SARIF object.
Writer->createRun("clang", Prefix);
}
@@ -43,8 +61,13 @@
assert(SARIFDiag && "SARIFDiagnostic has not been set.");
Writer->endRun();
llvm::json::Value Value(Writer->createDocument());
- OS << "\n" << Value << "\n\n";
- OS.flush();
+ getOS() << '\n' << Value << '\n';
+ getOS().flush();
+ if (EC and *EC) {
+ llvm::errs() << "error: " << EC->message() << '\n';
+ llvm::errs() << "outputting to stderr as a backup\n";
+ llvm::errs() << Value << '\n';
+ }
SARIFDiag.reset();
}
Index: clang/lib/Frontend/FrontendAction.cpp
===================================================================
--- clang/lib/Frontend/FrontendAction.cpp
+++ clang/lib/Frontend/FrontendAction.cpp
@@ -724,7 +724,9 @@
}
if (!CI.hasSourceManager()) {
CI.createSourceManager(CI.getFileManager());
- if (CI.getDiagnosticOpts().getFormat() == DiagnosticOptions::SARIF) {
+ if (TextDiagnosticFormat Format = CI.getDiagnosticOpts().getFormat();
+ Format == DiagnosticOptions::SARIFStderr ||
+ Format == DiagnosticOptions::SARIFFile) {
static_cast<SARIFDiagnosticPrinter *>(&CI.getDiagnosticClient())
->setSarifWriter(
std::make_unique<SarifDocumentWriter>(CI.getSourceManager()));
Index: clang/lib/Frontend/CompilerInstance.cpp
===================================================================
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -333,16 +333,18 @@
}
void CompilerInstance::createDiagnostics(DiagnosticConsumer *Client,
- bool ShouldOwnClient) {
- Diagnostics = createDiagnostics(&getDiagnosticOpts(), Client,
- ShouldOwnClient, &getCodeGenOpts());
-}
-
-IntrusiveRefCntPtr<DiagnosticsEngine>
-CompilerInstance::createDiagnostics(DiagnosticOptions *Opts,
- DiagnosticConsumer *Client,
- bool ShouldOwnClient,
- const CodeGenOptions *CodeGenOpts) {
+ bool ShouldOwnClient, bool ToFile) {
+ StringRef Input =
+ llvm::sys::path::filename(getFrontendOpts().Inputs.front().getFile());
+ StringRef Output = getFrontendOpts().OutputFile;
+ Diagnostics =
+ createDiagnostics(&getDiagnosticOpts(), Client, ShouldOwnClient,
+ &getCodeGenOpts(), Output.empty() ? Input : Output);
+}
+
+IntrusiveRefCntPtr<DiagnosticsEngine> CompilerInstance::createDiagnostics(
+ DiagnosticOptions *Opts, DiagnosticConsumer *Client, bool ShouldOwnClient,
+ const CodeGenOptions *CodeGenOpts, StringRef TargetPath) {
IntrusiveRefCntPtr<DiagnosticIDs> DiagID(new DiagnosticIDs());
IntrusiveRefCntPtr<DiagnosticsEngine>
Diags(new DiagnosticsEngine(DiagID, Opts));
@@ -351,8 +353,10 @@
// implementing -verify.
if (Client) {
Diags->setClient(Client, ShouldOwnClient);
- } else if (Opts->getFormat() == DiagnosticOptions::SARIF) {
+ } else if (Opts->getFormat() == DiagnosticOptions::SARIFStderr) {
Diags->setClient(new SARIFDiagnosticPrinter(llvm::errs(), Opts));
+ } else if (Opts->getFormat() == DiagnosticOptions::SARIFFile) {
+ Diags->setClient(new SARIFDiagnosticPrinter(TargetPath, Opts));
} else
Diags->setClient(new TextDiagnosticPrinter(llvm::errs(), Opts));
Index: clang/lib/Driver/ToolChains/Clang.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4032,8 +4032,7 @@
if (const Arg *A = Args.getLastArg(options::OPT_fdiagnostics_format_EQ)) {
CmdArgs.push_back("-fdiagnostics-format");
CmdArgs.push_back(A->getValue());
- if (StringRef(A->getValue()) == "sarif" ||
- StringRef(A->getValue()) == "SARIF")
+ if (StringRef(A->getValue()).starts_with("sarif"))
D.Diag(diag::warn_drv_sarif_format_unstable);
}
Index: clang/include/clang/Frontend/SARIFDiagnosticPrinter.h
===================================================================
--- clang/include/clang/Frontend/SARIFDiagnosticPrinter.h
+++ clang/include/clang/Frontend/SARIFDiagnosticPrinter.h
@@ -20,6 +20,8 @@
#include "llvm/ADT/IntrusiveRefCntPtr.h"
#include "llvm/ADT/StringRef.h"
#include <memory>
+#include <optional>
+#include <variant>
namespace clang {
class DiagnosticOptions;
@@ -30,6 +32,12 @@
class SARIFDiagnosticPrinter : public DiagnosticConsumer {
public:
SARIFDiagnosticPrinter(raw_ostream &OS, DiagnosticOptions *Diags);
+
+ /// Constructs a SARIFDiagnosticPrinter so that it writes to a file mimicing
+ /// Clang's current output target. The diagnostics file target has the same
+ /// name suffixed with `.sarif`.
+ SARIFDiagnosticPrinter(StringRef TargetPath, DiagnosticOptions *Diags);
+
~SARIFDiagnosticPrinter() = default;
SARIFDiagnosticPrinter &operator=(const SARIFDiagnosticPrinter &&) = delete;
@@ -59,7 +67,8 @@
const Diagnostic &Info) override;
private:
- raw_ostream &OS;
+ std::optional<std::error_code> EC;
+ std::variant<raw_ostream *, std::unique_ptr<raw_ostream>> OS;
IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts;
/// Handle to the currently active SARIF diagnostic emitter.
@@ -69,6 +78,16 @@
std::string Prefix;
std::unique_ptr<SarifDocumentWriter> Writer;
+
+ raw_ostream &getOS() {
+ // Deliberately not using std::visit due to there being exactly two known
+ // types.
+ if (std::holds_alternative<raw_ostream *>(OS)) {
+ return *std::get<raw_ostream *>(OS);
+ } else {
+ return *std::get<std::unique_ptr<raw_ostream>>(OS);
+ }
+ }
};
} // end namespace clang
Index: clang/include/clang/Frontend/CompilerInstance.h
===================================================================
--- clang/include/clang/Frontend/CompilerInstance.h
+++ clang/include/clang/Frontend/CompilerInstance.h
@@ -606,8 +606,10 @@
///
/// \param ShouldOwnClient If Client is non-NULL, specifies whether
/// the diagnostic object should take ownership of the client.
+ ///
+ /// \param ToFile Determines if Clang should write diagnostics to a file.
void createDiagnostics(DiagnosticConsumer *Client = nullptr,
- bool ShouldOwnClient = true);
+ bool ShouldOwnClient = true, bool ToFile = false);
/// Create a DiagnosticsEngine object with a the TextDiagnosticPrinter.
///
@@ -626,12 +628,16 @@
/// \param CodeGenOpts If non-NULL, the code gen options in use, which may be
/// used by some diagnostics printers (for logging purposes only).
///
+ /// \param TargetPath Path to the target that Clang is currently producing.
+ /// This will be used as the prefix of any file that Clang may write
+ /// diagnostics to (e.g. `-fdiagnostics-format=sarif-file -o /tmp/hello` would
+ /// lead to there being a file called `/tmp/hello.sarif`).
+ ///
/// \return The new object on success, or null on failure.
- static IntrusiveRefCntPtr<DiagnosticsEngine>
- createDiagnostics(DiagnosticOptions *Opts,
- DiagnosticConsumer *Client = nullptr,
- bool ShouldOwnClient = true,
- const CodeGenOptions *CodeGenOpts = nullptr);
+ static IntrusiveRefCntPtr<DiagnosticsEngine> createDiagnostics(
+ DiagnosticOptions *Opts, DiagnosticConsumer *Client = nullptr,
+ bool ShouldOwnClient = true, const CodeGenOptions *CodeGenOpts = nullptr,
+ StringRef TargetPath = "");
/// Create the file manager and replace any existing one with it.
///
Index: clang/include/clang/Driver/Options.td
===================================================================
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -5843,8 +5843,8 @@
def fdiagnostics_format : Separate<["-"], "fdiagnostics-format">,
HelpText<"Change diagnostic formatting to match IDE and command line tools">,
- Values<"clang,msvc,vi,sarif,SARIF">,
- NormalizedValuesScope<"DiagnosticOptions">, NormalizedValues<["Clang", "MSVC", "Vi", "SARIF", "SARIF"]>,
+ Values<"clang,msvc,vi,sarif-stderr,sarif-file">,
+ NormalizedValuesScope<"DiagnosticOptions">, NormalizedValues<["Clang", "MSVC", "Vi", "SARIFStderr", "SARIFFile"]>,
MarshallingInfoEnum<DiagnosticOpts<"Format">, "Clang">;
def fdiagnostics_show_category : Separate<["-"], "fdiagnostics-show-category">,
HelpText<"Print diagnostic category">,
Index: clang/include/clang/Basic/DiagnosticOptions.h
===================================================================
--- clang/include/clang/Basic/DiagnosticOptions.h
+++ clang/include/clang/Basic/DiagnosticOptions.h
@@ -74,7 +74,7 @@
friend class CompilerInvocation;
public:
- enum TextDiagnosticFormat { Clang, MSVC, Vi, SARIF };
+ enum TextDiagnosticFormat { Clang, MSVC, Vi, SARIFStderr, SARIFFile };
// Default values.
enum {
Index: clang/include/clang/Basic/DiagnosticOptions.def
===================================================================
--- clang/include/clang/Basic/DiagnosticOptions.def
+++ clang/include/clang/Basic/DiagnosticOptions.def
@@ -63,7 +63,7 @@
VALUE_DIAGOPT(ShowCategories, 2, 0) /// Show categories: 0 -> none, 1 -> Number,
/// 2 -> Full Name.
-ENUM_DIAGOPT(Format, TextDiagnosticFormat, 2, Clang) /// Format for diagnostics:
+ENUM_DIAGOPT(Format, TextDiagnosticFormat, 3, Clang) /// Format for diagnostics:
DIAGOPT(ShowColors, 1, 0) /// Show diagnostics with ANSI color sequences.
DIAGOPT(UseANSIEscapeCodes, 1, 0)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
