[clang] adds prototype for being able to alternate diagnostic formats

[cjdb]

Summary

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

[cjdb]

arc diff no longer seems to autoformat

[aaron.ballman]

clang/include/clang/Basic/Diagnostic.h

212–214

To make sure I understand, notes are one context, but are error/warning/remarks another context, and same for fixits?

I'm having a hard time understanding how you expect to use this; in the diagnostics engine, notes are like any other diagnostic, so are you going to look at the diagnostic ID and dispatch to either the BasicDiagnostic or the DiagnosticContext interface? I'd appreciate a bit more information on how this separation works.

1024

Question: should we have the overload set so that you get a const writer on a const engine and a non-const writer on a non-const engine?

[cjdb]

Abandoning since this design is obsolete.

clang/include/clang/Basic/Diagnostic.h

 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_BASIC_DIAGNOSTIC_H
 #define LLVM_CLANG_BASIC_DIAGNOSTIC_H
 
 #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"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
struct DiagnosticStorage {
 
   /// If valid, provides a hint with some code to insert, remove, or
   /// modify at a particular position.
   SmallVector<FixItHint, 6> FixItHints;
 
   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
 /// as errors" and passes them off to the DiagnosticConsumer for reporting to
 /// the user. DiagnosticsEngine is tied to one translation unit and one
 /// SourceManager.
 class DiagnosticsEngine : public RefCountedBase<DiagnosticsEngine> {
 public:
private:
   // Cap on depth of constexpr evaluation backtrace stack, 0 -> no limit.
   unsigned ConstexprBacktraceLimit = 0;
 
   IntrusiveRefCntPtr<DiagnosticIDs> Diags;
   IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts;
   DiagnosticConsumer *Client = nullptr;
   std::unique_ptr<DiagnosticConsumer> Owner;
   SourceManager *SourceMgr = nullptr;
+  SarifDocumentWriter *SarifWriter = nullptr;
 
   /// Mapping information for diagnostics.
   ///
   /// Mapping info is packed into four bits per diagnostic.  The low three
   /// bits are the mapping (an instance of diag::Severity), or zero if unset.
   /// The high bit is set when the mapping was established as a user mapping.
   /// If the high bit is clear, then the low bits are set to the default
   /// value, and should be mapped with -pedantic, -Werror, etc.
void SetDelayedDiagnostic(unsigned DiagID, StringRef Arg1 = "",
                             StringRef Arg2 = "", StringRef Arg3 = "");
 
   /// Clear out the current diagnostic.
   void Clear() { CurDiagID = std::numeric_limits<unsigned>::max(); }
 
   /// 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
   // object.  This implementation choice means that we can only have one
   // diagnostic "in flight" at a time, but this seems to be a reasonable
   // tradeoff to keep these objects small.  Assertions verify that only one
   // diagnostic is in flight at a time.
   friend class Diagnostic;
public:
 
   template <typename T> const DiagnosticBuilder &operator<<(const T &V) const {
     assert(isActive() && "Clients must not add to cleared diagnostic!");
     const StreamingDiagnostic &DB = *this;
     DB << V;
     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.
   template <typename T, typename = typename std::enable_if<
                             !std::is_lvalue_reference<T>::value>::type>
   const DiagnosticBuilder &operator<<(T &&V) const {
     assert(isActive() && "Clients must not add to cleared diagnostic!");
     const StreamingDiagnostic &DB = *this;

clang/include/clang/Sema/Sema.h

 #include "clang/AST/PrettyPrinter.h"
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtOpenMP.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/AST/TypeOrdering.h"
 #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"
 #include "clang/Basic/OpenMPKinds.h"
 #include "clang/Basic/PragmaKinds.h"
 #include "clang/Basic/Specifiers.h"
 #include "clang/Basic/TemplateKinds.h"
 #include "clang/Basic/TypeTraits.h"
operator<<(const SemaDiagnosticBuilder &Diag, const T &Value) {
       if (Diag.ImmediateDiag)
         *Diag.ImmediateDiag << Value;
       else if (Diag.PartialDiagId)
         Diag.S.DeviceDeferredDiags[Diag.Fn][*Diag.PartialDiagId].second
             << Value;
       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.
     template <typename T, typename = typename std::enable_if<
                               !std::is_lvalue_reference<T>::value>::type>
     const SemaDiagnosticBuilder &operator<<(T &&V) const {
       if (ImmediateDiag)
         *ImmediateDiag << std::move(V);
struct LateParsedTemplate {
   Decl *D;
 };
 
 template <>
 void Sema::PragmaStack<Sema::AlignPackInfo>::Act(SourceLocation PragmaLocation,
                                                  PragmaMsStackAction Action,
                                                  llvm::StringRef StackSlotLabel,
                                                  AlignPackInfo Value);
-
 } // end namespace clang
 
 namespace llvm {
 // Hash a FunctionDeclAndLoc by looking at both its FunctionDecl and its
 // SourceLocation.
 template <> struct DenseMapInfo<clang::Sema::FunctionDeclAndLoc> {
   using FunctionDeclAndLoc = clang::Sema::FunctionDeclAndLoc;
   using FDBaseInfo = DenseMapInfo<clang::CanonicalDeclPtr<clang::FunctionDecl>>;

clang/unittests/Basic/SarifTest.cpp

const SarifResult &Result = SarifResult::create(RuleIdx)
                                   .setThreadFlows(Threadflows);
   Writer.appendResult(Result);
   std::string Output = serializeSarifDocument(Writer.createDocument());
 
   // THEN: Assert that the serialized SARIF is as expected
   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