cjdb created this revision.
cjdb added reviewers: aaron.ballman, erichkeane, shafik, dblaikie.
Herald added a project: All.
cjdb requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.
The original `-fdiagnostics-format=sarif` wrote directly to standard
error. This commit renames that mode to `sarif-stderr` and introduces a
`sarif-file` mode that allows the compiler to write `.sarif` files to
the current working directory. It also removes `SARIF` as an option so
that there's a single canonical approach for each option.
IMPORTANT: There are two different designs for writing SARIF to file,
so I've pulled the common parts of the design out into their own patch.
This ensures it only gets reviewed once, and so that the divergence is
apparent in the other two patches. Once a design is settled on, this
patch will be joined with the selected design.
Repository:
rG LLVM Github Monorepo
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));
@@ -1221,7 +1225,6 @@
&ImportingInstance.getModuleCache());
auto &Inv = *Invocation;
Instance.setInvocation(std::move(Invocation));
-
Instance.createDiagnostics(new ForwardingDiagnosticConsumer(
ImportingInstance.getDiagnosticClient()),
/*ShouldOwnClient=*/true);
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,12 @@
///
/// \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);
+
+ std::string MakeInputPath() const;
/// Create a DiagnosticsEngine object with a the TextDiagnosticPrinter.
///
@@ -626,12 +630,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
