cjdb created this revision.
cjdb added a reviewer: aaron.ballman.
Herald added a project: All.
cjdb requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
In an effort to move from unstructured text diagnostics to structured
diagnostics, while keeping support for the existing model, we need to
introduce a way to hotswap between the desired modes. The visitor
pattern is a natural ally here because it allows us to defer logic to
handlers that know the specifics of the diagnostic without cluttering
the compiler's logic.
There are two parts to the design: `BasicDiagnostic`, which is used as
an abstract base for errors, warnings, and remarks; and
`DiagnosticContext`, which is used as an abstract base for notes. We've
split diagnostics into these two categories so that it's possible for to
enrich diagnostics with multiple unrelated contexts (notes seem to trail
diagnostics in a detatched and unherded fashion). FixIts are a form of
diagnostic context.
Although parts of `BasicDiagnostic` and `DiagnosticContext` should be
generated via TableGen, we're initially implementing everything manually
so that we can prototype what is necessary: it will be far easier to do
this with a handful of diagnostics than to also figure out how to
integrate this into TableGen at the same time.
**Reviewers should note that testing is currently lacking. Tests for the
diagnostic framework seem to be nonexistent; this should probably
change.**
Depends on: D109701 <https://reviews.llvm.org/D109701>
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D129538
Files:
clang/include/clang/Basic/Diagnostic.h
clang/include/clang/Sema/Sema.h
clang/unittests/Basic/SarifTest.cpp
Index: clang/unittests/Basic/SarifTest.cpp
===================================================================
--- clang/unittests/Basic/SarifTest.cpp
+++ clang/unittests/Basic/SarifTest.cpp
@@ -317,4 +317,20 @@
ASSERT_THAT(Output, ::testing::StrEq(ExpectedOutput));
}
+// Check that we can set/get a SarifDocumentWriter from the DiagnosticsEngine
+// Note that there doesn't appear to be any tests for the DiagnosticsEngine, so
+// we test here.
+TEST_F(SarifDocumentWriterTest, checkDiagnosticsEngineSetAndGet) {
+ ASSERT_FALSE(Diags.getSarifWriter());
+
+ // GIVEN:
+ SarifDocumentWriter Writer(SourceMgr);
+
+ // WHEN: the DiagnosticsEngine is set
+ Diags.setSarifWriter(&Writer);
+
+ // THEN: the DiagnosticsEngine points to the local SarifDocumentWriter
+ EXPECT_TRUE(Diags.getSarifWriter() == &Writer);
+}
+
} // namespace
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -38,6 +38,7 @@
#include "clang/Basic/BitmaskEnum.h"
#include "clang/Basic/Builtins.h"
#include "clang/Basic/DarwinSDKInfo.h"
+#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/ExpressionTraits.h"
#include "clang/Basic/Module.h"
#include "clang/Basic/OpenCLOptions.h"
@@ -1772,6 +1773,14 @@
return Diag;
}
+ const SemaDiagnosticBuilder &operator<<(const BasicDiagnostic &Diag) const {
+ if (SarifDocumentWriter *W = S.Diags.getSarifWriter())
+ Diag.EmitSarifDiagnostic(*W);
+ else
+ Diag.EmitTraditionalDiagnostic();
+ return *this;
+ }
+
// It is necessary to limit this to rvalue reference to avoid calling this
// function with a bitfield lvalue argument since non-const reference to
// bitfield is not allowed.
@@ -13584,6 +13593,28 @@
llvm::StringRef StackSlotLabel,
AlignPackInfo Value);
+class SemaDiagnostic : public BasicDiagnostic {
+protected:
+ SemaDiagnostic(DiagID ID, Sema& S)
+ : BasicDiagnostic(ID)
+ , S(&S)
+ {}
+
+ Sema& getSema() const { return *S; }
+private:
+ Sema* S;
+};
+
+class SemaDiagnosticContext : public DiagnosticContext {
+protected:
+ explicit SemaDiagnosticContext(Sema& S)
+ : S(&S)
+ {}
+
+ Sema& getSema() const { return *S; }
+private:
+ Sema* S;
+};
} // end namespace clang
namespace llvm {
Index: clang/include/clang/Basic/Diagnostic.h
===================================================================
--- clang/include/clang/Basic/Diagnostic.h
+++ clang/include/clang/Basic/Diagnostic.h
@@ -16,6 +16,7 @@
#include "clang/Basic/DiagnosticIDs.h"
#include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Basic/Sarif.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/Specifiers.h"
#include "llvm/ADT/ArrayRef.h"
@@ -183,6 +184,45 @@
DiagnosticStorage() = default;
};
+/// Abstract interface for all errors, warnings, and remarks. This allows us to
+/// use the visitor pattern. This is something that will be concretely defined
+/// in the short-term, as we experiment, but will be migrated over to TableGen
+/// after a few diagnostics have been implemented.
+class BasicDiagnostic {
+public:
+ virtual ~BasicDiagnostic() = default;
+
+ /// Emits the diagnostic as Clang's original unstructured text format.
+ virtual void EmitTraditionalDiagnostic() const = 0;
+
+ /// Emits the diagnostic as SARIF.
+ virtual void EmitSarifDiagnostic(SarifDocumentWriter &) const = 0;
+protected:
+ using DiagID = unsigned;
+
+ explicit BasicDiagnostic(DiagID K)
+ : ID(K)
+ {}
+
+ DiagID getID() const { return ID; }
+private:
+ DiagID ID;
+};
+
+/// Abstract interface for diagnostic contexts (traditionally: notes).
+/// A BasicDiagnostic may have as many different contexts as required to provide
+/// users with a complete picture.
+class DiagnosticContext {
+public:
+ virtual ~DiagnosticContext() = default;
+
+ /// Emits the diagnostic as an unstructured text note.
+ virtual void EmitTraditionalNote() const = 0;
+
+ /// Emits the diagnostic as SARIF.
+ virtual void EmitSarifContext(SarifDocumentWriter &) const = 0;
+};
+
/// Concrete class used by the front-end to report problems and issues.
///
/// This massages the diagnostics (e.g. handling things like "report warnings
@@ -295,6 +335,7 @@
DiagnosticConsumer *Client = nullptr;
std::unique_ptr<DiagnosticConsumer> Owner;
SourceManager *SourceMgr = nullptr;
+ SarifDocumentWriter *SarifWriter = nullptr;
/// Mapping information for diagnostics.
///
@@ -979,6 +1020,9 @@
/// Return the value associated with this diagnostic flag.
StringRef getFlagValue() const { return FlagValue; }
+ void setSarifWriter(SarifDocumentWriter *W) { SarifWriter = W; }
+ SarifDocumentWriter *getSarifWriter() const { return SarifWriter; }
+
private:
// This is private state used by DiagnosticBuilder. We put it here instead of
// in DiagnosticBuilder in order to keep DiagnosticBuilder a small lightweight
@@ -1343,6 +1387,15 @@
return *this;
}
+ const DiagnosticBuilder &operator<<(const BasicDiagnostic &Diag) const {
+ assert(isActive() && "Clients must not add to cleared diagnostic!");
+ if (SarifDocumentWriter *W = DiagObj->getSarifWriter())
+ Diag.EmitSarifDiagnostic(*W);
+ else
+ Diag.EmitTraditionalDiagnostic();
+ return *this;
+ }
+
// It is necessary to limit this to rvalue reference to avoid calling this
// function with a bitfield lvalue argument since non-const reference to
// bitfield is not allowed.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
