[FileCheck][NFC] Refactor FileCheckDiag into class hierarchy

[jdenny]

Summary

Problem

FileCheckDiag and its enum MatchType have outgrown their original purpose. The -dump-input presentation layer (in llvm/utils/FileCheck/FileCheck.cpp) and the FileCheck library's diagnostic emission (in llvm/lib/FileCheck/FileCheck.cpp) are too tightly coupled. The interactions are subtle to understand and maintain. It is difficult for the former to reason about the latter's emitted diagnostics in order to present them in the most readable manner.

Solution

This patch removes MatchType from FileCheckDiag and refactors FileCheckDiag as the base class of a class hierarchy. That class hierarchy is designed to enable the FileCheck library to focus on communicating FileCheck diagnostic information clearly and completely without participating in the specific presentation decisions of -dump-input. -dump-input is then freer to evolve more independently in the way it reasons about emitted diagnostics.

[jdenny]

Below are some more details that seem worthwhile to capture somewhere
but that seem like too much for the review summary.

Background

Over a year ago in D96653, when we realized that patch was not a good direction, we discussed a new way to mark a FileCheck directive's search range in input dumps. I've tinkered with that idea since then and concluded that FileCheckDiag first needs a rewrite, which is the goal of the current patch.

Several months ago, I finished implementing the search-range idea in subsequent patches, and I have been observing the effect on FileCheck's input dumps since then to be sure I'm happy with it. Eventually I'll post the additional patches to see what others think.

My point in mentioning the search-range work here is that it offers some evidence that the FileCheckDiag rewrite in the current patch is usable for further evolution of input dumps. However, I think this rewrite is worthwhile for code readability and maintainability even if the search-range idea is ultimately not accepted by the community.

Specific Issues Addressed

Result vs. Note

Similar to compiler errors/warnings/remarks vs. notes, the FileCheckDiag series emitted by the FileCheck library contains match results, each of which might be followed by a series of associated notes before the next match result. Without this patch, that association is not formally modeled by FileCheckDiag or clearly documented, and -dump-input isn't able to easily reason about it.

Specifically, without being aware of that association, -dump-input needs some other means to determine (1) whether some notes should be included by -dump-input-filter=error (the default filter), and (2) where to place some notes in the input dump. As a result, the FileCheck library is responsible for copying the following from a match result to any note that doesn't have its own versions: (1) the MatchType, which determines error status, and (2) the input range so that -dump-input can place the note next to the match result. This copied information is redundant as it's already part of the associated match result, and semantically it doesn't really apply to the note itself.

Furthermore, for a note that does not have a match location (e.g., the FileCheck library doesn't include match locations when emitting notes about variable substitutions), the FileCheck library is responsible for reducing the copied location to an empty range as a special case that -dump-input understands means it should not add a marker (like ^~~), which would be misleading in the input dump. However, that means a note can never have a real input range that is empty and that thus refers to just a single position.

These interactions are too subtle and are hard to explain. As a thought experiment to improve the separation of semantics from presentation, imagine an alternative HTML-based presentation (this is purely hypothetical: I have no plans to develop this). Imagine notes appear in pop-up windows when clicking on match results. It is then useless for the FileCheck library to copy the error status and location from a match result to its notes. However, the association between the match result and its notes *is* relevant in *both* the HTML-based presentation and in the existing -dump-input presentation.

This patch formally models the relationship between match results and notes with two classes directly derived from FileCheckDiag: MatchResultDiag and MatchNoteDiag. A MatchNoteDiag does not carry a bogus MatchTy, and a match range is exposed as an Optional. Thus, this patch relieves the FileCheck library of the above -dump-input-specific responsibilities, and -dump-input is now able to cleanly examine the FileCheckDiag series for each required property.

getMarker

Because of the FileCheckDiag class hierarchy, this patch is able to clean up the getMarker function for -dump-input. Without this patch, getMarker encodes the lead character, color, message, and error status individually for every possible MatchType. With this patch, getMarker instead encodes the logic of how the marker properties are generally chosen at the top of the FileCheckDiag hierarchy, and then it overrides those choices where needed lower in the hierarchy. Again, it is now easier for a diagnostic presentation layer to reason about diagnostics.

Diagnostic-Specific Data

Because of the class hierarchy, each FileCheckDiag can contain just the information that is useful for its kind of diagnostic. MatchNoteDiag does not need to store a FileCheckType or check location as that information can be retrieved from the associated MatchResultDiag. MatchCustomNoteDiag is the only class that needs to store a note string. MatchFoundDiag now records both a match range and a search range. Currently, -dump-input does not make use of the search range for MatchFoundDiag, but I plan to change that in the case of errors, as discussed above.

llvm/include/llvm/FileCheck/FileCheck.h

 //==-- llvm/FileCheck/FileCheck.h --------------------------------*- C++ -*-==//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 /// \file This file has some utilities to use FileCheck as an API
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_FILECHECK_FILECHECK_H
 #define LLVM_FILECHECK_FILECHECK_H
 
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Casting.h"
 #include "llvm/Support/Regex.h"
 #include "llvm/Support/SMLoc.h"
 #include <bitset>
 #include <memory>
 #include <string>
 #include <vector>
 
 namespace llvm {
public:
   // \returns a description of \p Prefix.
   std::string getDescription(StringRef Prefix) const;
 
   // \returns a description of \p Modifiers.
   std::string getModifiersDescription() const;
 };
 } // namespace Check
 
-/// Summary of a FileCheck diagnostic.
-struct FileCheckDiag {
-  /// What is the FileCheck directive for this diagnostic?
+class MatchResultDiag;
+
+/// Abstract base class for recording a FileCheck diagnostic for a pattern
+/// (e.g., \c CHECK-NEXT directive or \c --implicit-check-not).
+///
+/// \c FileCheckDiag has two direct derived classes:
+/// - \c MatchResultDiag records a match result for a pattern.  There might be
+///   more than one for a single pattern.  For example, there might be several
+///   discarded matches before either a good match or a failure to match.
+/// - \c MatchNoteDiag provides an additional note about the most recent
+///   \c MatchResultDiag emitted by a FileCheck invocation.  For example, there
+///   might be a fuzzy match after a failure to match.
+///
+/// Throughout this class hierarchy, a pattern is said to be either expected or
+/// excluded depending on whether the pattern must have or must not have a match
+/// in order for it to succeed.  For example, a \c CHECK directive's pattern is
+/// expected, and a \c CHECK-NOT directive's pattern is excluded.
+class FileCheckDiag {
+public:
+  enum FileCheckDiagKind {
+    // MatchResultDiag
+    FCDK_MatchResultDiag_First,
+    FCDK_MatchFoundDiag = FCDK_MatchResultDiag_First,
+    FCDK_MatchNoneDiag,
+    FCDK_MatchResultDiag_Last = FCDK_MatchNoneDiag,
+    // MatchNoteDiag
+    FCDK_MatchNoteDiag_First,
+    FCDK_MatchFuzzyDiag = FCDK_MatchNoteDiag_First,
+    FCDK_MatchCustomNoteDiag,
+    FCDK_MatchNoteDiag_Last = FCDK_MatchCustomNoteDiag
+  };
+
+private:
+  const FileCheckDiagKind Kind;
+
+public:
+  FileCheckDiag(FileCheckDiagKind Kind) : Kind(Kind) {}
+  /// Destructor is purely virtual to ensure this remains an abstract class.
+  virtual ~FileCheckDiag() = 0;
+  /// Of what derived class is this an instance?
+  FileCheckDiagKind getKind() const { return Kind; }
+  /// If this is a \c MatchResultDiag, return itself.  If this is a
+  /// \c MatchNoteDiag, return its associated \c MatchResultDiag.
+  virtual const MatchResultDiag &getMatchResultDiag() const = 0;
+  /// Does this diagnostic reveal an error?
+  ///
+  /// For \c MatchResultDiag, \c !isError() is not always the same as a
+  /// successful pattern match result.  For \c MatchNoteDiag, \c !isError()
+  /// does not indicate the lack of an error but rather the lack of an
+  /// additional error beyond its associated \c MatchResultDiag.  See
+  /// documentation on derived types for details.
+  virtual bool isError() const = 0;
+  /// Return the input range for which this diagnostic indicates text that was
+  /// matched in some way (e.g., successful pattern match, discarded pattern
+  /// match, or variable capture), or return \c llvm::None if the diagnostic has
+  /// no such input range.
+  virtual Optional<SMRange> getMatchRange() const = 0;
+};
+
+/// Abstract base class for recording a FileCheck diagnostic that reports a
+/// match result for a pattern.
+class MatchResultDiag : public FileCheckDiag {
+private:
   Check::FileCheckType CheckTy;
-  /// Where is the FileCheck directive for this diagnostic?
   SMLoc CheckLoc;
-  /// What type of match result does this diagnostic describe?
-  ///
-  /// A directive's supplied pattern is said to be either expected or excluded
-  /// depending on whether the pattern must have or must not have a match in
-  /// order for the directive to succeed.  For example, a CHECK directive's
-  /// pattern is expected, and a CHECK-NOT directive's pattern is excluded.
-  ///
-  /// There might be more than one match result for a single pattern.  For
-  /// example, there might be several discarded matches
-  /// (MatchFoundButDiscarded) before either a good match
-  /// (MatchFoundAndExpected) or a failure to match (MatchNoneButExpected),
-  /// and there might be a fuzzy match (MatchFuzzy) after the latter.
-  enum MatchType {
+  SMRange SearchRange;
+
+public:
+  MatchResultDiag(FileCheckDiagKind Kind, const Check::FileCheckType &CheckTy,
+                  SMLoc CheckLoc, SMRange SearchRange)
+      : FileCheckDiag(Kind), CheckTy(CheckTy), CheckLoc(CheckLoc),
+        SearchRange(SearchRange) {}
+  /// Destructor is purely virtual to ensure this remains an abstract class.
+  virtual ~MatchResultDiag() = 0;
+  /// Is \p FCD an instance of \c MatchResultDiag?
+  static bool classof(const FileCheckDiag *FCD) {
+    FileCheckDiagKind FCDK = FCD->getKind();
+    return FCDK_MatchResultDiag_First <= FCDK &&
+           FCDK <= FCDK_MatchResultDiag_Last;
+  }
+  /// Get itself.
+  virtual const MatchResultDiag &getMatchResultDiag() const { return *this; }
+  /// What is the type of pattern for this match result?
+  Check::FileCheckType getCheckTy() const { return CheckTy; }
+  /// Where is the pattern for this match result?
+  SMLoc getCheckLoc() const { return CheckLoc; }
+  /// What is the search range for the match result?
+  SMRange getSearchRange() const { return SearchRange; }
+};
+
+/// \c MatchResultDiag for a pattern that matched the input.
+class MatchFoundDiag : public MatchResultDiag {
+public:
+  enum StatusTy {
     /// Indicates a good match for an expected pattern.
-    MatchFoundAndExpected,
-    /// Indicates a match for an excluded pattern.
-    MatchFoundButExcluded,
+    Success,
+    /// Indicates a match for an excluded pattern (error).
+    Excluded,
     /// Indicates a match for an expected pattern, but the match is on the
-    /// wrong line.
-    MatchFoundButWrongLine,
-    /// Indicates a discarded match for an expected pattern.
-    MatchFoundButDiscarded,
-    /// Indicates an error while processing a match after the match was found
-    /// for an expected or excluded pattern.  The error is specified by \c Note,
-    /// to which it should be appropriate to prepend "error: " later.  The full
-    /// match itself should be recorded in a preceding diagnostic of a different
-    /// \c MatchFound match type.
-    MatchFoundErrorNote,
+    /// wrong line (error).
+    WrongLine,
+    /// Indicates a discarded match for an expected pattern (not an error).
+    Discarded
+  };
+
+private:
+  StatusTy Status;
+  SMRange MatchRange;
+
+public:
+  MatchFoundDiag(const Check::FileCheckType &CheckTy, SMLoc CheckLoc,
+                 StatusTy Status, SMRange MatchRange, SMRange SearchRange)
+      : MatchResultDiag(FCDK_MatchFoundDiag, CheckTy, CheckLoc, SearchRange),
+        Status(Status), MatchRange(MatchRange) {}
+  /// Is \p FCD an instance of \c MatchFoundDiag?
+  static bool classof(const FileCheckDiag *FCD) {
+    return FCD->getKind() == FCDK_MatchFoundDiag;
+  }
+  /// Does this match produce an error?
+  ///
+  /// This is not always the same as \c getStatus()!=Success.  For example,
+  /// \c CHECK-DAG discarded matches are neither successful matches nor errors.
+  virtual bool isError() const {
+    return Status != Success && Status != Discarded;
+  }
+  /// Was this a successful match?  If not, why not?
+  ///
+  /// See \c isError comments for the relationship between the two.
+  StatusTy getStatus() const { return Status; }
+  /// Adjust a successful status to a non-successful status.
+  ///
+  /// This is designed to be called while emitting diagnostics.  It is not
+  /// designed to be called by a diagnostic presentation layer like
+  /// `-dump-input`.
+  ///
+  /// For example, a match that was originally thought to be successful might
+  /// later be discarded, or it might be determined that it violates a matching
+  /// constraint (e.g., wrong line).
+  void markUnsuccessful(StatusTy S) {
+    assert(Status == Success && S != Success &&
+           "expected to change successful status to unsuccessful");
+    Status = S;
+  }
+  /// Return the match's input range, never \c llvm::None.
+  virtual Optional<SMRange> getMatchRange() const { return MatchRange; }
+};
+
+/// \c MatchResultDiag for a pattern that did not match the input.
+class MatchNoneDiag : public MatchResultDiag {
+public:
+  enum StatusTy {
     /// Indicates no match for an excluded pattern.
-    MatchNoneAndExcluded,
-    /// Indicates no match for an expected pattern, but this might follow good
-    /// matches when multiple matches are expected for the pattern, or it might
-    /// follow discarded matches for the pattern.
-    MatchNoneButExpected,
+    Success,
     /// Indicates no match due to an expected or excluded pattern that has
-    /// proven to be invalid at match time.  The exact problems are usually
-    /// reported in subsequent diagnostics of the same match type but with
-    /// \c Note set.
-    MatchNoneForInvalidPattern,
-    /// Indicates a fuzzy match that serves as a suggestion for the next
-    /// intended match for an expected pattern with too few or no good matches.
-    MatchFuzzy,
-  } MatchTy;
-  /// The search range if MatchTy starts with MatchNone, or the match range
-  /// otherwise.
-  unsigned InputStartLine;
-  unsigned InputStartCol;
-  unsigned InputEndLine;
-  unsigned InputEndCol;
-  /// A note to replace the one normally indicated by MatchTy, or the empty
-  /// string if none.
+    /// proven to be invalid at match time (error).  The exact problems are
+    /// usually reported in subsequent \c MatchNoteDiag objects.
+    InvalidPattern,
+    /// Indicates no match for an expected pattern (error).  This might follow
+    /// good matches when multiple matches are expected for the pattern, or it
+    /// might follow discarded matches for the pattern.
+    Expected
+  };
+
+private:
+  StatusTy Status;
+
+public:
+  MatchNoneDiag(const Check::FileCheckType &CheckTy, SMLoc CheckLoc,
+                StatusTy Status, SMRange SearchRange)
+      : MatchResultDiag(FCDK_MatchNoneDiag, CheckTy, CheckLoc, SearchRange),
+        Status(Status) {}
+  /// Is \p FCD an instance of \c MatchNoneDiag?
+  static bool classof(const FileCheckDiag *FCD) {
+    return FCD->getKind() == FCDK_MatchNoneDiag;
+  }
+  /// Does the lack of match represent an error?
+  virtual bool isError() const { return Status != Success; }
+  /// Does the lack of a match indicate a success?  If not, why not?
+  StatusTy getStatus() const { return Status; }
+  /// Return \c llvm::None.
+  virtual Optional<SMRange> getMatchRange() const { return None; }
+};
+
+/// Abstract base class for recording a FileCheck diagnostic that provides an
+/// additional note (possibly an additional error) about the most recent
+/// \c MatchResultDiag.
+class MatchNoteDiag : public FileCheckDiag {
+private:
+  MatchResultDiag *MRD;
+
+public:
+  MatchNoteDiag(FileCheckDiagKind FCDK) : FileCheckDiag(FCDK), MRD(nullptr) {}
+  /// Destructor is purely virtual to ensure this remains an abstract class.
+  virtual ~MatchNoteDiag() = 0;
+  /// Is \p FCD an instance of \c MatchNoteDiag?
+  static bool classof(const FileCheckDiag *FCD) {
+    FileCheckDiagKind FCDK = FCD->getKind();
+    return FCDK_MatchNoteDiag_First <= FCDK && FCDK <= FCDK_MatchNoteDiag_Last;
+  }
+  /// Get the note's associated \c MatchResultDiag.
+  virtual const MatchResultDiag &getMatchResultDiag() const { return *MRD; }
+  /// Set the note's associated \c MatchResultDiag.
+  void setMatchResultDiag(MatchResultDiag *MRDNew) {
+    assert(!MRD && "expected setMatchResultDiag to be called only once");
+    MRD = MRDNew;
+  }
+};
+
+/// \c MatchNoteDiag for a fuzzy match that serves as a suggestion for the next
+class MatchFuzzyDiag : public MatchNoteDiag {
+private:
+  SMLoc MatchStart;
+
+public:
+  MatchFuzzyDiag(SMLoc MatchStart)
+      : MatchNoteDiag(FCDK_MatchFuzzyDiag), MatchStart(MatchStart) {}
+  /// Is \p FCD an instance of \c MatchFuzzyDiag?
+  static bool classof(const FileCheckDiag *FCD) {
+    return FCD->getKind() == FCDK_MatchFuzzyDiag;
+  }
+  /// Always false.  A fuzzy match is not an error even though it is performed
+  /// due to an error.
+  virtual bool isError() const { return false; }
+  /// Return an input range (never \c llvm::None) starting and ending at the
+  /// match start.  The actual match end is not computed.
+  virtual Optional<SMRange> getMatchRange() const {
+    return SMRange(MatchStart, MatchStart);
+  }
+};
+
+/// \c MatchNoteDiag with a custom note not described by any other class derived
+/// from \c MatchNoteDiag.
+class MatchCustomNoteDiag : public MatchNoteDiag {
+private:
   std::string Note;
-  FileCheckDiag(const SourceMgr &SM, const Check::FileCheckType &CheckTy,
-                SMLoc CheckLoc, MatchType MatchTy, SMRange InputRange,
-                StringRef Note = "");
+  bool AddsError;
+  Optional<SMRange> MatchRange;
+
+public:
+  /// If \p MatchRange is specified, it is a range for input text that was
+  /// matched in some way (e.g., variable capture) and that is described by
+  /// this note.  Either way, as usual, the associated \c MatchResultDiag has
+  /// any full match range for the pattern.
+  ///
+  /// If \p AddsError is true, then this note indicates an \a additional error
+  /// that is distinct from any error indicated by the associated
+  /// \c MatchResultDiag.  The error is described by \c Note, which must be
+  /// worded appropriately for prepending "error: " when presented later.  For
+  /// example, the associated \c MatchResultDiag indicates a match to either an
+  /// expected pattern (success) or an excluded pattern (error), and \c Note
+  /// is "unable to represent numeric value" to indicate the match could not be
+  /// processed afterward.
+  ///
+  /// If \p AddsError is false, then this note merely provides additional
+  /// information about the associated \c MatchResultDiag.  That information
+  /// might be something harmless (e.g., variable substitution), or it might be
+  /// one of potentially many problems summarized as an error by the
+  /// \c MatchResultDiag (e.g., one way in which the pattern was invalid).
+  ///@{
+  MatchCustomNoteDiag(SMRange MatchRange, StringRef Note,
+                      bool AddsError = false)
+      : MatchNoteDiag(FCDK_MatchCustomNoteDiag), Note(Note),
+        AddsError(AddsError), MatchRange(MatchRange) {}
+  MatchCustomNoteDiag(StringRef Note)
+      : MatchNoteDiag(FCDK_MatchCustomNoteDiag), Note(Note), AddsError(false) {}
+  ///@}
+  /// Is \p FCD an instance of \c MatchCustomNoteDiag?
+  static bool classof(const FileCheckDiag *FCD) {
+    return FCD->getKind() == FCDK_MatchCustomNoteDiag;
+  }
+  const std::string &getNote() const { return Note; }
+  /// Does this note indicate an \a additional error not indicated by the
+  /// associated \c MatchResultDiag?
+  ///
+  /// For details, see the \c MatchCustomNoteDiag::MatchCustomNoteDiag comments
+  /// for its \c AddsError parameter.
+  virtual bool isError() const { return AddsError; }
+  /// Return the match range described by the note, or \c llvm::None if none.
+  virtual Optional<SMRange> getMatchRange() const { return MatchRange; }
+};
+
+/// A \c FileCheckDiag series emitted by the FileCheck library.
+class FileCheckDiagList {
+private:
+  MatchResultDiag *CurMatchResultDiag = nullptr;
+  std::vector<std::unique_ptr<FileCheckDiag>> DiagList;
+
+public:
+  /// Emplace a new \c FileCheckDiag of type \c DiagTy.  If it's a
+  /// \c MatchNoteDiag, associate it with its \c MatchResultDiag.
+  ///
+  /// \c FileCheckTest.cpp calls \c Pattern::printVariableDefs directly, so it
+  /// can add a \c MatchNoteDiag without a previous \c MatchResultDiag.
+  /// Otherwise, there should always be a previous \c MatchResultDiag.
+  template <typename DiagTy, typename... ArgTys>
+  void emplace(const ArgTys &...Args) {
+    DiagList.emplace_back(std::make_unique<DiagTy>(Args...));
+    FileCheckDiag *Diag = DiagList.back().get();
+    if (MatchResultDiag *MRD = dyn_cast<MatchResultDiag>(Diag)) {
+      CurMatchResultDiag = MRD;
+      return;
+    }
+    MatchNoteDiag *Note = cast<MatchNoteDiag>(Diag);
+    if (!CurMatchResultDiag)
+      return;
+    Note->setMatchResultDiag(CurMatchResultDiag);
+  }
+  /// Adjust the previous \c MatchResultDiag, which must be a \c MatchFoundDiag,
+  /// from successful status to unsuccessful status.
+  void adjustPrevMatchFoundDiag(MatchFoundDiag::StatusTy Status) {
+    cast<MatchFoundDiag>(CurMatchResultDiag)->markUnsuccessful(Status);
+  }
+  /// The \c FileCheckDiag list.
+  const std::vector<std::unique_ptr<FileCheckDiag>> &getList() const {
+    return DiagList;
+  }
 };
 
 class FileCheckPatternContext;
 struct FileCheckString;
 
 /// FileCheck class takes the request and exposes various methods that
 /// use information from the request.
 class FileCheck {
StringRef CanonicalizeFile(MemoryBuffer &MB,
                              SmallVectorImpl<char> &OutputBuffer);
 
   /// Checks the input to FileCheck provided in the \p Buffer against the
   /// expected strings read from the check file and record diagnostics emitted
   /// in \p Diags. Errors are recorded against \p SM.
   ///
   /// \returns false if the input fails to satisfy the checks.
   bool checkInput(SourceMgr &SM, StringRef Buffer,
-                  std::vector<FileCheckDiag> *Diags = nullptr);
+                  FileCheckDiagList *Diags = nullptr);
 };
 
 } // namespace llvm
 
 #endif

llvm/lib/FileCheck/FileCheck.cpp

unsigned Pattern::computeMatchDistance(StringRef Buffer) const {
   // Only compare up to the first line in the buffer, or the string size.
   StringRef BufferPrefix = Buffer.substr(0, ExampleString.size());
   BufferPrefix = BufferPrefix.split('\n').first;
   return BufferPrefix.edit_distance(ExampleString);
 }
 
 void Pattern::printSubstitutions(const SourceMgr &SM, StringRef Buffer,
                                  SMRange Range,
-                                 FileCheckDiag::MatchType MatchTy,
-                                 std::vector<FileCheckDiag> *Diags) const {
+                                 FileCheckDiagList *Diags) const {
   // Print what we know about substitutions.
   if (!Substitutions.empty()) {
     for (const auto &Substitution : Substitutions) {
       SmallString<256> Msg;
       raw_svector_ostream OS(Msg);
 
       Expected<std::string> MatchedValue = Substitution->getResult();
       // Substitution failures are handled in printNoMatch().
       if (!MatchedValue) {
         consumeError(MatchedValue.takeError());
         continue;
       }
 
       OS << "with \"";
       OS.write_escaped(Substitution->getFromString()) << "\" equal to \"";
       OS.write_escaped(*MatchedValue) << "\"";
 
-      // We report only the start of the match/search range to suggest we are
-      // reporting the substitutions as set at the start of the match/search.
-      // Indicating a non-zero-length range might instead seem to imply that the
+      // Unlike MatchCustomNoteDiag, PrintMessage needs a location.  We report
+      // only the start of the match/search range to suggest we are reporting
+      // the substitutions as set at the start of the match/search.  Indicating
+      // a non-zero-length range might instead seem to imply that the
       // substitution matches or was captured from exactly that range.
       if (Diags)
-        Diags->emplace_back(SM, CheckTy, getLoc(), MatchTy,
-                            SMRange(Range.Start, Range.Start), OS.str());
+        Diags->emplace<MatchCustomNoteDiag>(OS.str());
       else
         SM.PrintMessage(Range.Start, SourceMgr::DK_Note, OS.str());
     }
   }
 }
 
 void Pattern::printVariableDefs(const SourceMgr &SM,
-                                FileCheckDiag::MatchType MatchTy,
-                                std::vector<FileCheckDiag> *Diags) const {
+                                FileCheckDiagList *Diags) const {
   if (VariableDefs.empty() && NumericVariableDefs.empty())
     return;
   // Build list of variable captures.
   struct VarCapture {
     StringRef Name;
     SMRange Range;
   };
   SmallVector<VarCapture, 2> VarCaptures;
llvm::sort(VarCaptures, [](const VarCapture &A, const VarCapture &B) {
     return A.Range.Start.getPointer() < B.Range.Start.getPointer();
   });
   // Create notes for the sorted captures.
   for (const VarCapture &VC : VarCaptures) {
     SmallString<256> Msg;
     raw_svector_ostream OS(Msg);
     OS << "captured var \"" << VC.Name << "\"";
     if (Diags)
-      Diags->emplace_back(SM, CheckTy, getLoc(), MatchTy, VC.Range, OS.str());
+      Diags->emplace<MatchCustomNoteDiag>(VC.Range, OS.str());
     else
       SM.PrintMessage(VC.Range.Start, SourceMgr::DK_Note, OS.str(), VC.Range);
   }
 }
 
-static SMRange ProcessMatchResult(FileCheckDiag::MatchType MatchTy,
-                                  const SourceMgr &SM, SMLoc Loc,
-                                  Check::FileCheckType CheckTy,
-                                  StringRef Buffer, size_t Pos, size_t Len,
-                                  std::vector<FileCheckDiag> *Diags,
-                                  bool AdjustPrevDiags = false) {
+static SMRange buildMatchRange(StringRef Buffer, size_t Pos, size_t Len) {
   SMLoc Start = SMLoc::getFromPointer(Buffer.data() + Pos);
   SMLoc End = SMLoc::getFromPointer(Buffer.data() + Pos + Len);
-  SMRange Range(Start, End);
-  if (Diags) {
-    if (AdjustPrevDiags) {
-      SMLoc CheckLoc = Diags->rbegin()->CheckLoc;
-      for (auto I = Diags->rbegin(), E = Diags->rend();
-           I != E && I->CheckLoc == CheckLoc; ++I)
-        I->MatchTy = MatchTy;
-    } else
-      Diags->emplace_back(SM, CheckTy, Loc, MatchTy, Range);
-  }
-  return Range;
+  return SMRange(Start, End);
+
+static SMRange buildSearchRange(StringRef Buffer) {
+  SMLoc Start = SMLoc::getFromPointer(Buffer.data());
+  SMLoc End = SMLoc::getFromPointer(Buffer.data() + Buffer.size());
+  return SMRange(Start, End);
 }
 
 void Pattern::printFuzzyMatch(const SourceMgr &SM, StringRef Buffer,
-                              std::vector<FileCheckDiag> *Diags) const {
+                              FileCheckDiagList *Diags) const {
   // Attempt to find the closest/best fuzzy match.  Usually an error happens
   // because some string in the output didn't exactly match. In these cases, we
   // would like to show the user a best guess at what "should have" matched, to
   // save them having to actually check the input manually.
   size_t NumLinesForward = 0;
   size_t Best = StringRef::npos;
   double BestQuality = 0;
 
if (Quality < BestQuality || Best == StringRef::npos) {
       BestQuality = Quality;
     }
   }
 
   // Print the "possible intended match here" line if we found something
   // reasonable and not equal to what we showed in the "scanning from here"
   // line.
   if (Best && Best != StringRef::npos && BestQuality < 50) {
-    SMRange MatchRange =
-        ProcessMatchResult(FileCheckDiag::MatchFuzzy, SM, getLoc(),
-                           getCheckTy(), Buffer, Best, 0, Diags);
-    SM.PrintMessage(MatchRange.Start, SourceMgr::DK_Note,
+    SMLoc MatchStart = SMLoc::getFromPointer(Buffer.data() + Best);
+    if (Diags)
+      Diags->emplace<MatchFuzzyDiag>(MatchStart);
+    SM.PrintMessage(MatchStart, SourceMgr::DK_Note,
                     "possible intended match here");
 
     // FIXME: If we wanted to be really friendly we would show why the match
     // failed, as it can be hard to spot simple one character differences.
   }
 }
 
 Expected<StringRef>
while (Ptr + 1 != End && (Ptr[1] == ' ' || Ptr[1] == '\t'))
       ++Ptr;
   }
 
   // Add a null byte and then return all but that byte.
   OutputBuffer.push_back('\0');
   return StringRef(OutputBuffer.data(), OutputBuffer.size() - 1);
 }
 
-FileCheckDiag::FileCheckDiag(const SourceMgr &SM,
-                             const Check::FileCheckType &CheckTy,
-                             SMLoc CheckLoc, MatchType MatchTy,
-                             SMRange InputRange, StringRef Note)
-    : CheckTy(CheckTy), CheckLoc(CheckLoc), MatchTy(MatchTy), Note(Note) {
-  auto Start = SM.getLineAndColumn(InputRange.Start);
-  auto End = SM.getLineAndColumn(InputRange.End);
-  InputStartLine = Start.first;
-  InputStartCol = Start.second;
-  InputEndLine = End.first;
-  InputEndCol = End.second;
-}
+FileCheckDiag::~FileCheckDiag() {}
+MatchResultDiag::~MatchResultDiag() {}
+MatchNoteDiag::~MatchNoteDiag() {}
 
 static bool IsPartOfWord(char c) {
   return (isAlnum(c) || c == '-' || c == '_');
 }
 
 Check::FileCheckType &Check::FileCheckType::setCount(int C) {
   assert(Count > 0 && "zero and negative counts are not supported");
   assert((C == 1 || Kind == CheckPlain) &&

 }
 
 /// Returns either (1) \c ErrorSuccess if there was no error or (2)
 /// \c ErrorReported if an error was reported, such as an unexpected match.
 static Error printMatch(bool ExpectedMatch, const SourceMgr &SM,
                         StringRef Prefix, SMLoc Loc, const Pattern &Pat,
                         int MatchedCount, StringRef Buffer,
                         Pattern::MatchResult MatchResult,
-                        const FileCheckRequest &Req,
-                        std::vector<FileCheckDiag> *Diags) {
+                        const FileCheckRequest &Req, FileCheckDiagList *Diags) {
   // Suppress some verbosity if there's no error.
   bool HasError = !ExpectedMatch || MatchResult.TheError;
   bool PrintDiag = true;
   if (!HasError) {
     if (!Req.Verbose)
       return ErrorReported::reportedOrSuccess(HasError);
     if (!Req.VerboseVerbose && Pat.getCheckTy() == Check::CheckEOF)
       return ErrorReported::reportedOrSuccess(HasError);
     // Due to their verbosity, we don't print verbose diagnostics here if we're
     // gathering them for Diags to be rendered elsewhere, but we always print
     // other diagnostics.
     PrintDiag = !Diags;
   }
 
   // Add "found" diagnostic, substitutions, and variable definitions to Diags.
-  FileCheckDiag::MatchType MatchTy = ExpectedMatch
-                                         ? FileCheckDiag::MatchFoundAndExpected
-                                         : FileCheckDiag::MatchFoundButExcluded;
-  SMRange MatchRange = ProcessMatchResult(MatchTy, SM, Loc, Pat.getCheckTy(),
-                                          Buffer, MatchResult.TheMatch->Pos,
-                                          MatchResult.TheMatch->Len, Diags);
+  MatchFoundDiag::StatusTy Status =
+      ExpectedMatch ? MatchFoundDiag::Success : MatchFoundDiag::Excluded;
+  SMRange MatchRange = buildMatchRange(Buffer, MatchResult.TheMatch->Pos,
+                                       MatchResult.TheMatch->Len);
+  SMRange SearchRange = buildSearchRange(Buffer);
   if (Diags) {
-    Pat.printSubstitutions(SM, Buffer, MatchRange, MatchTy, Diags);
-    Pat.printVariableDefs(SM, MatchTy, Diags);
+    Diags->emplace<MatchFoundDiag>(Pat.getCheckTy(), Loc, Status, MatchRange,
+                                   SearchRange);
+    Pat.printSubstitutions(SM, Buffer, MatchRange, Diags);
+    Pat.printVariableDefs(SM, Diags);
   }
   if (!PrintDiag) {
     assert(!HasError && "expected to report more diagnostics for error");
     return ErrorReported::reportedOrSuccess(HasError);
   }
 
   // Print the match.
   std::string Message = formatv("{0}: {1} string found in input",
                                 Pat.getCheckTy().getDescription(Prefix),
                                 (ExpectedMatch ? "expected" : "excluded"))
                             .str();
   if (Pat.getCount() > 1)
     Message += formatv(" ({0} out of {1})", MatchedCount, Pat.getCount()).str();
   SM.PrintMessage(
       Loc, ExpectedMatch ? SourceMgr::DK_Remark : SourceMgr::DK_Error, Message);
   SM.PrintMessage(MatchRange.Start, SourceMgr::DK_Note, "found here",
                   {MatchRange});
 
   // Print additional information, which can be useful even if there are errors.
-  Pat.printSubstitutions(SM, Buffer, MatchRange, MatchTy, nullptr);
-  Pat.printVariableDefs(SM, MatchTy, nullptr);
+  Pat.printSubstitutions(SM, Buffer, MatchRange, nullptr);
+  Pat.printVariableDefs(SM, nullptr);
 
   // Print errors and add them to Diags.  We report these errors after the match
   // itself because we found them after the match.  If we had found them before
   // the match, we'd be in printNoMatch.
   handleAllErrors(std::move(MatchResult.TheError),
                   [&](const ErrorDiagnostic &E) {
                     E.log(errs());
                     if (Diags) {
-                      Diags->emplace_back(SM, Pat.getCheckTy(), Loc,
-                                          FileCheckDiag::MatchFoundErrorNote,
-                                          E.getRange(), E.getMessage().str());
+                      Diags->emplace<MatchCustomNoteDiag>(E.getRange(),
+                                                          E.getMessage().str(),
+                                                          /*AddsError=*/true);
                     }
                   });
   return ErrorReported::reportedOrSuccess(HasError);
 }
 
 /// Returns either (1) \c ErrorSuccess if there was no error, or (2)
 /// \c ErrorReported if an error was reported, such as an expected match not
 /// found.
 static Error printNoMatch(bool ExpectedMatch, const SourceMgr &SM,
                           StringRef Prefix, SMLoc Loc, const Pattern &Pat,
                           int MatchedCount, StringRef Buffer, Error MatchError,
-                          bool VerboseVerbose,
-                          std::vector<FileCheckDiag> *Diags) {
+                          bool VerboseVerbose, FileCheckDiagList *Diags) {
   // Print any pattern errors, and record them to be added to Diags later.
   bool HasError = ExpectedMatch;
   bool HasPatternError = false;
-  FileCheckDiag::MatchType MatchTy = ExpectedMatch
-                                         ? FileCheckDiag::MatchNoneButExpected
-                                         : FileCheckDiag::MatchNoneAndExcluded;
+  MatchNoneDiag::StatusTy Status =
+      ExpectedMatch ? MatchNoneDiag::Expected : MatchNoneDiag::Success;
   SmallVector<std::string, 4> ErrorMsgs;
   handleAllErrors(
       std::move(MatchError),
       [&](const ErrorDiagnostic &E) {
         HasError = HasPatternError = true;
-        MatchTy = FileCheckDiag::MatchNoneForInvalidPattern;
+        Status = MatchNoneDiag::InvalidPattern;
         E.log(errs());
         if (Diags)
           ErrorMsgs.push_back(E.getMessage().str());
       },
       // NotFoundError is why printNoMatch was invoked.
       [](const NotFoundError &E) {});
 
   // Suppress some verbosity if there's no error.
static Error printNoMatch(bool ExpectedMatch, const SourceMgr &SM,
 
   // Add "not found" diagnostic, substitutions, and pattern errors to Diags.
   //
   // We handle Diags a little differently than the errors we print directly:
   // we add the "not found" diagnostic to Diags even if there are pattern
   // errors.  The reason is that we need to attach pattern errors as notes
   // somewhere in the input, and the input search range from the "not found"
   // diagnostic is all we have to anchor them.
-  SMRange SearchRange = ProcessMatchResult(MatchTy, SM, Loc, Pat.getCheckTy(),
-                                           Buffer, 0, Buffer.size(), Diags);
+  SMRange SearchRange = buildSearchRange(Buffer);
   if (Diags) {
-    SMRange NoteRange = SMRange(SearchRange.Start, SearchRange.Start);
+    Diags->emplace<MatchNoneDiag>(Pat.getCheckTy(), Loc, Status, SearchRange);
     for (StringRef ErrorMsg : ErrorMsgs)
-      Diags->emplace_back(SM, Pat.getCheckTy(), Loc, MatchTy, NoteRange,
-                          ErrorMsg);
-    Pat.printSubstitutions(SM, Buffer, SearchRange, MatchTy, Diags);
+      Diags->emplace<MatchCustomNoteDiag>(ErrorMsg);
+    Pat.printSubstitutions(SM, Buffer, SearchRange, Diags);
   }
   if (!PrintDiag) {
     assert(!HasError && "expected to report more diagnostics for error");
     return ErrorReported::reportedOrSuccess(HasError);
   }
 
   // Print "not found" diagnostic, except that's implied if we already printed a
   // pattern error.
SM.PrintMessage(Loc,
                     ExpectedMatch ? SourceMgr::DK_Error : SourceMgr::DK_Remark,
                     Message);
     SM.PrintMessage(SearchRange.Start, SourceMgr::DK_Note,
                     "scanning from here");
   }
 
   // Print additional information, which can be useful even after a pattern
   // error.
-  Pat.printSubstitutions(SM, Buffer, SearchRange, MatchTy, nullptr);
+  Pat.printSubstitutions(SM, Buffer, SearchRange, nullptr);
   if (ExpectedMatch)
     Pat.printFuzzyMatch(SM, Buffer, Diags);
   return ErrorReported::reportedOrSuccess(HasError);
 }
 
 /// Returns either (1) \c ErrorSuccess if there was no error, or (2)
 /// \c ErrorReported if an error was reported.
 static Error reportMatchResult(bool ExpectedMatch, const SourceMgr &SM,
                                StringRef Prefix, SMLoc Loc, const Pattern &Pat,
                                int MatchedCount, StringRef Buffer,
                                Pattern::MatchResult MatchResult,
                                const FileCheckRequest &Req,
-                               std::vector<FileCheckDiag> *Diags) {
+                               FileCheckDiagList *Diags) {
   if (MatchResult.TheMatch)
     return printMatch(ExpectedMatch, SM, Prefix, Loc, Pat, MatchedCount, Buffer,
                       std::move(MatchResult), Req, Diags);
   return printNoMatch(ExpectedMatch, SM, Prefix, Loc, Pat, MatchedCount, Buffer,
                       std::move(MatchResult.TheError), Req.VerboseVerbose,
                       Diags);
 }
 
while (true) {
     if (NumNewLines == 1)
       FirstNewLine = Range.begin();
   }
 }
 
 size_t FileCheckString::Check(const SourceMgr &SM, StringRef Buffer,
                               bool IsLabelScanMode, size_t &MatchLen,
                               FileCheckRequest &Req,
-                              std::vector<FileCheckDiag> *Diags) const {
+                              FileCheckDiagList *Diags) const {
   size_t LastPos = 0;
   std::vector<const Pattern *> NotStrings;
 
   // IsLabelScanMode is true when we are scanning forward to find CHECK-LABEL
   // bounds; we have not processed variable definitions within the bounded block
   // yet so cannot handle any final CHECK-DAG yet; this is handled when going
   // over the block again (including the last CHECK-LABEL) in normal mode.
   if (!IsLabelScanMode) {
size_t FileCheckString::Check(const SourceMgr &SM, StringRef Buffer,
   if (!IsLabelScanMode) {
     size_t MatchPos = FirstMatchPos - LastPos;
     StringRef MatchBuffer = Buffer.substr(LastPos);
     StringRef SkippedRegion = Buffer.substr(LastPos, MatchPos);
 
     // If this check is a "CHECK-NEXT", verify that the previous match was on
     // the previous line (i.e. that there is one newline between them).
     if (CheckNext(SM, SkippedRegion)) {
-      ProcessMatchResult(FileCheckDiag::MatchFoundButWrongLine, SM, Loc,
-                         Pat.getCheckTy(), MatchBuffer, MatchPos, MatchLen,
-                         Diags, Req.Verbose);
+      if (Diags) {
+        if (Req.Verbose)
+          Diags->adjustPrevMatchFoundDiag(MatchFoundDiag::WrongLine);
+        else {
+          SMRange MatchRange = buildMatchRange(MatchBuffer, MatchPos, MatchLen);
+          SMRange SearchRange = buildSearchRange(MatchBuffer);
+          Diags->emplace<MatchFoundDiag>(Pat.getCheckTy(), Loc,
+                                         MatchFoundDiag::WrongLine, MatchRange,
+                                         SearchRange);
+        }
+      }
       return StringRef::npos;
     }
 
     // If this check is a "CHECK-SAME", verify that the previous match was on
     // the same line (i.e. that there is no newline between them).
     if (CheckSame(SM, SkippedRegion)) {
-      ProcessMatchResult(FileCheckDiag::MatchFoundButWrongLine, SM, Loc,
-                         Pat.getCheckTy(), MatchBuffer, MatchPos, MatchLen,
-                         Diags, Req.Verbose);
+      if (Diags) {
+        if (Req.Verbose)
+          Diags->adjustPrevMatchFoundDiag(MatchFoundDiag::WrongLine);
+        else {
+          SMRange MatchRange = buildMatchRange(MatchBuffer, MatchPos, MatchLen);
+          SMRange SearchRange = buildSearchRange(MatchBuffer);
+          Diags->emplace<MatchFoundDiag>(Pat.getCheckTy(), Loc,
+                                         MatchFoundDiag::WrongLine, MatchRange,
+                                         SearchRange);
+        }
+      }
       return StringRef::npos;
     }
 
     // If this match had "not strings", verify that they don't exist in the
     // skipped region.
     if (CheckNot(SM, SkippedRegion, NotStrings, Req, Diags))
       return StringRef::npos;
   }
bool FileCheckString::CheckSame(const SourceMgr &SM, StringRef Buffer) const {
   }
 
   return false;
 }
 
 bool FileCheckString::CheckNot(const SourceMgr &SM, StringRef Buffer,
                                const std::vector<const Pattern *> &NotStrings,
                                const FileCheckRequest &Req,
-                               std::vector<FileCheckDiag> *Diags) const {
+                               FileCheckDiagList *Diags) const {
   bool DirectiveFail = false;
   for (const Pattern *Pat : NotStrings) {
     assert((Pat->getCheckTy() == Check::CheckNot) && "Expect CHECK-NOT!");
     Pattern::MatchResult MatchResult = Pat->match(Buffer, SM);
     if (Error Err = reportMatchResult(/*ExpectedMatch=*/false, SM, Prefix,
                                       Pat->getLoc(), *Pat, 1, Buffer,
                                       std::move(MatchResult), Req, Diags)) {
       cantFail(handleErrors(std::move(Err), [&](const ErrorReported &E) {}));
       DirectiveFail = true;
       continue;
     }
   }
   return DirectiveFail;
 }
 
 size_t FileCheckString::CheckDag(const SourceMgr &SM, StringRef Buffer,
                                  std::vector<const Pattern *> &NotStrings,
                                  const FileCheckRequest &Req,
-                                 std::vector<FileCheckDiag> *Diags) const {
+                                 FileCheckDiagList *Diags) const {
   if (DagNotStrings.empty())
     return 0;
 
   // The start of the search range.
   size_t StartPos = 0;
 
   struct MatchRange {
     size_t Pos;
for (auto MI = MatchRanges.begin(), ME = MatchRanges.end(); true; ++MI) {
         // Insert non-overlapping match into list.
         MatchRanges.insert(MI, M);
         break;
       }
       if (Req.VerboseVerbose) {
         // Due to their verbosity, we don't print verbose diagnostics here if
         // we're gathering them for a different rendering, but we always print
         // other diagnostics.
-        if (!Diags) {
+        if (Diags)
+          Diags->adjustPrevMatchFoundDiag(MatchFoundDiag::Discarded);
+        else {
           SMLoc OldStart = SMLoc::getFromPointer(Buffer.data() + MI->Pos);
           SMLoc OldEnd = SMLoc::getFromPointer(Buffer.data() + MI->End);
           SMRange OldRange(OldStart, OldEnd);
           SM.PrintMessage(OldStart, SourceMgr::DK_Note,
                           "match discarded, overlaps earlier DAG match here",
                           {OldRange});
-        } else {
-          SMLoc CheckLoc = Diags->rbegin()->CheckLoc;
-          for (auto I = Diags->rbegin(), E = Diags->rend();
-               I != E && I->CheckLoc == CheckLoc; ++I)
-            I->MatchTy = FileCheckDiag::MatchFoundButDiscarded;
         }
       }
       MatchPos = MI->End;
     }
     if (!Req.VerboseVerbose)
       cantFail(printMatch(
           /*ExpectedMatch=*/true, SM, Prefix, Pat.getLoc(), Pat, 1, Buffer,
           Pattern::MatchResult(MatchPos, MatchLen, Error::success()), Req,
void FileCheckPatternContext::clearLocalVars() {
 
   for (const auto &Var : LocalPatternVars)
     GlobalVariableTable.erase(Var);
   for (const auto &Var : LocalNumericVars)
     GlobalNumericVariableTable.erase(Var);
 }
 
 bool FileCheck::checkInput(SourceMgr &SM, StringRef Buffer,
-                           std::vector<FileCheckDiag> *Diags) {
+                           FileCheckDiagList *Diags) {
   bool ChecksFailed = false;
 
   unsigned i = 0, j = 0, e = CheckStrings->size();
   while (true) {
     StringRef CheckRegion;
     if (j == e) {
       CheckRegion = Buffer;
     } else {

llvm/lib/FileCheck/FileCheckImpl.h

public:
   /// instance provides the current values of FileCheck string variables and is
   /// updated if this match defines new values. Likewise, the
   /// GlobalNumericVariableTable StringMap in the same class provides the
   /// current values of FileCheck numeric variables and is updated if this
   /// match defines new numeric values.
   MatchResult match(StringRef Buffer, const SourceMgr &SM) const;
   /// Prints the value of successful substitutions.
   void printSubstitutions(const SourceMgr &SM, StringRef Buffer,
-                          SMRange MatchRange, FileCheckDiag::MatchType MatchTy,
-                          std::vector<FileCheckDiag> *Diags) const;
+                          SMRange MatchRange, FileCheckDiagList *Diags) const;
   void printFuzzyMatch(const SourceMgr &SM, StringRef Buffer,
-                       std::vector<FileCheckDiag> *Diags) const;
+                       FileCheckDiagList *Diags) const;
 
   bool hasVariable() const {
     return !(Substitutions.empty() && VariableDefs.empty());
   }
-  void printVariableDefs(const SourceMgr &SM, FileCheckDiag::MatchType MatchTy,
-                         std::vector<FileCheckDiag> *Diags) const;
+  void printVariableDefs(const SourceMgr &SM, FileCheckDiagList *Diags) const;
 
   Check::FileCheckType getCheckTy() const { return CheckTy; }
 
   int getCount() const { return CheckTy.getCount(); }
 
 private:
   bool AddRegExToRegEx(StringRef RS, unsigned &CurParen, SourceMgr &SM);
   void AddBackrefToRegEx(unsigned BackrefNum);
struct FileCheckString {
   std::vector<Pattern> DagNotStrings;
 
   FileCheckString(const Pattern &P, StringRef S, SMLoc L)
       : Pat(P), Prefix(S), Loc(L) {}
 
   /// Matches check string and its "not strings" and/or "dag strings".
   size_t Check(const SourceMgr &SM, StringRef Buffer, bool IsLabelScanMode,
                size_t &MatchLen, FileCheckRequest &Req,
-               std::vector<FileCheckDiag> *Diags) const;
+               FileCheckDiagList *Diags) const;
 
   /// Verifies that there is a single line in the given \p Buffer. Errors are
   /// reported against \p SM.
   bool CheckNext(const SourceMgr &SM, StringRef Buffer) const;
   /// Verifies that there is no newline in the given \p Buffer. Errors are
   /// reported against \p SM.
   bool CheckSame(const SourceMgr &SM, StringRef Buffer) const;
   /// Verifies that none of the strings in \p NotStrings are found in the given
   /// \p Buffer. Errors are reported against \p SM and diagnostics recorded in
   /// \p Diags according to the verbosity level set in \p Req.
   bool CheckNot(const SourceMgr &SM, StringRef Buffer,
                 const std::vector<const Pattern *> &NotStrings,
-                const FileCheckRequest &Req,
-                std::vector<FileCheckDiag> *Diags) const;
+                const FileCheckRequest &Req, FileCheckDiagList *Diags) const;
   /// Matches "dag strings" and their mixed "not strings".
   size_t CheckDag(const SourceMgr &SM, StringRef Buffer,
                   std::vector<const Pattern *> &NotStrings,
-                  const FileCheckRequest &Req,
-                  std::vector<FileCheckDiag> *Diags) const;
+                  const FileCheckRequest &Req, FileCheckDiagList *Diags) const;
 };
 
 } // namespace llvm
 
 #endif

llvm/unittests/FileCheck/FileCheckTest.cpp

public:
   Expected<size_t> match(StringRef Buffer) {
     StringRef BufferRef = bufferize(SM, Buffer);
     Pattern::MatchResult Res = P.match(BufferRef, SM);
     if (Res.TheError)
       return std::move(Res.TheError);
     return Res.TheMatch->Pos;
   }
 
-  void printVariableDefs(FileCheckDiag::MatchType MatchTy,
-                         std::vector<FileCheckDiag> &Diags) {
-    P.printVariableDefs(SM, MatchTy, &Diags);
+  void printVariableDefs(FileCheckDiagList &Diags) {
+    P.printVariableDefs(SM, &Diags);
   }
+
+  SourceMgr &getSourceMgr() { return SM; }
 };
 
+#define EXPECT_SM_RANGE(SM, RangeOpt, StartLineExpected, StartColExpected,     \
+                        EndLineExpected, EndColExpected)                       \
+  do {                                                                         \
+    EXPECT_TRUE(RangeOpt->isValid());                                          \
+    SMRange Range = RangeOpt.getValue();                                       \
+    auto StartActual = SM.getLineAndColumn(Range.Start);                       \
+    auto EndActual = SM.getLineAndColumn(Range.End);                           \
+    unsigned StartLineActual = StartActual.first;                              \
+    unsigned StartColActual = StartActual.second;                              \
+    unsigned EndLineActual = EndActual.first;                                  \
+    unsigned EndColActual = EndActual.second;                                  \
+    EXPECT_EQ(StartLineActual, StartLineExpected);                             \
+    EXPECT_EQ(StartColActual, StartColExpected);                               \
+    EXPECT_EQ(EndLineActual, EndLineExpected);                                 \
+    EXPECT_EQ(EndColActual, EndColExpected);                                   \
+  } while (0)
+
 TEST_F(FileCheckTest, ParseNumericSubstitutionBlock) {
   PatternTester Tester;
 
   // Variable definition.
 
   expectDiagnosticError("invalid variable name",
                         Tester.parseSubst("%VAR:").takeError());
 
#endif
   ASSERT_THAT_EXPECTED(ExpressionPointer, Succeeded());
   ExpressionVal = (*ExpressionPointer)->getAST()->eval();
   ASSERT_THAT_EXPECTED(ExpressionVal, Succeeded());
   EXPECT_EQ(cantFail(ExpressionVal->getSignedValue()), 36);
 }
 
 TEST_F(FileCheckTest, CapturedVarDiags) {
   PatternTester Tester;
+  SourceMgr &SM = Tester.getSourceMgr();
   ASSERT_FALSE(Tester.parsePattern("[[STRVAR:[a-z]+]] [[#NUMVAR:@LINE]]"));
   EXPECT_THAT_EXPECTED(Tester.match("foobar 2"), Succeeded());
-  std::vector<FileCheckDiag> Diags;
-  Tester.printVariableDefs(FileCheckDiag::MatchFoundAndExpected, Diags);
-  EXPECT_EQ(Diags.size(), 2ul);
-  for (FileCheckDiag Diag : Diags) {
-    EXPECT_EQ(Diag.CheckTy, Check::CheckPlain);
-    EXPECT_EQ(Diag.MatchTy, FileCheckDiag::MatchFoundAndExpected);
-    EXPECT_EQ(Diag.InputStartLine, 1u);
-    EXPECT_EQ(Diag.InputEndLine, 1u);
-  }
-  EXPECT_EQ(Diags[0].InputStartCol, 1u);
-  EXPECT_EQ(Diags[0].InputEndCol, 7u);
-  EXPECT_EQ(Diags[1].InputStartCol, 8u);
-  EXPECT_EQ(Diags[1].InputEndCol, 9u);
-  EXPECT_EQ(Diags[0].Note, "captured var \"STRVAR\"");
-  EXPECT_EQ(Diags[1].Note, "captured var \"NUMVAR\"");
+  FileCheckDiagList Diags;
+  Tester.printVariableDefs(Diags);
+  EXPECT_EQ(Diags.getList().size(), 2ul);
+  for (const std::unique_ptr<FileCheckDiag> &Diag : Diags.getList()) {
+    EXPECT_EQ(Diag->getKind(), FileCheckDiag::FCDK_MatchCustomNoteDiag);
+    EXPECT_FALSE(Diag->isError());
+  }
+  EXPECT_SM_RANGE(SM, Diags.getList()[0]->getMatchRange(), 1u, 1u, 1u, 7u);
+  EXPECT_SM_RANGE(SM, Diags.getList()[1]->getMatchRange(), 1u, 8u, 1u, 9u);
+  EXPECT_EQ(cast<MatchCustomNoteDiag>(Diags.getList()[0].get())->getNote(),
+            "captured var \"STRVAR\"");
+  EXPECT_EQ(cast<MatchCustomNoteDiag>(Diags.getList()[1].get())->getNote(),
+            "captured var \"NUMVAR\"");
 }
 } // namespace

llvm/utils/FileCheck/FileCheck.cpp

struct MarkerStyle {
   /// The starting char (before tildes) for marking the line.
   char Lead;
   /// What color to use for this annotation.
   raw_ostream::Colors Color;
   /// A note to follow the marker, or empty string if none.
   std::string Note;
   /// Does this marker indicate inclusion by -dump-input-filter=error?
   bool FiltersAsError;
-  MarkerStyle() {}
-  MarkerStyle(char Lead, raw_ostream::Colors Color,
-              const std::string &Note = "", bool FiltersAsError = false)
-      : Lead(Lead), Color(Color), Note(Note), FiltersAsError(FiltersAsError) {
-    assert((!FiltersAsError || !Note.empty()) &&
-           "expected error diagnostic to have note");
-  }
 };
 
-static MarkerStyle GetMarker(FileCheckDiag::MatchType MatchTy) {
-  switch (MatchTy) {
-  case FileCheckDiag::MatchFoundAndExpected:
-    return MarkerStyle('^', raw_ostream::GREEN);
-  case FileCheckDiag::MatchFoundButExcluded:
-    return MarkerStyle('!', raw_ostream::RED, "error: no match expected",
-                       /*FiltersAsError=*/true);
-  case FileCheckDiag::MatchFoundButWrongLine:
-    return MarkerStyle('!', raw_ostream::RED, "error: match on wrong line",
-                       /*FiltersAsError=*/true);
-  case FileCheckDiag::MatchFoundButDiscarded:
-    return MarkerStyle('!', raw_ostream::CYAN,
-                       "discard: overlaps earlier match");
-  case FileCheckDiag::MatchFoundErrorNote:
-    // Note should always be overridden within the FileCheckDiag.
-    return MarkerStyle('!', raw_ostream::RED,
-                       "error: unknown error after match",
-                       /*FiltersAsError=*/true);
-  case FileCheckDiag::MatchNoneAndExcluded:
-    return MarkerStyle('X', raw_ostream::GREEN);
-  case FileCheckDiag::MatchNoneButExpected:
-    return MarkerStyle('X', raw_ostream::RED, "error: no match found",
-                       /*FiltersAsError=*/true);
-  case FileCheckDiag::MatchNoneForInvalidPattern:
-    return MarkerStyle('X', raw_ostream::RED,
-                       "error: match failed for invalid pattern",
-                       /*FiltersAsError=*/true);
-  case FileCheckDiag::MatchFuzzy:
-    return MarkerStyle('?', raw_ostream::MAGENTA, "possible intended match",
-                       /*FiltersAsError=*/true);
+static MarkerStyle getMarker(const FileCheckDiag *Diag) {
+  // By default, the marker is based on whether the diagnostic is an error or is
+  // a MatchNoteDiag on a MatchResultDiag that is an error.
+  //
+  // It's less confusing if diagnostics that don't actually have match ranges
+  // don't have markers.  For example, a marker for the MatchNoteDiag
+  // 'with "VAR" equal to "5"' would seem to indicate where "VAR" matches, but
+  // we don't actually have that location.  Instead, we just place the note
+  // after the start of the associated MatchResultDiag.  This decision is
+  // overriden below for the case of MatchNoneDiag because the search range is
+  // used instead.
+  MarkerStyle Res;
+  bool IsError = Diag->isError() || Diag->getMatchResultDiag().isError();
+  Res.Lead = !Diag->getMatchRange().hasValue() ? ' ' : IsError ? '!' : '^';
+  Res.Color = IsError ? raw_ostream::RED : raw_ostream::GREEN;
+  Res.FiltersAsError = IsError;
+
+  // Override the default for some kinds of diagnostics.
+  switch (Diag->getKind()) {
+  case FileCheckDiag::FCDK_MatchFoundDiag:
+    switch (cast<MatchFoundDiag>(Diag)->getStatus()) {
+    case MatchFoundDiag::Success:
+      break;
+    case MatchFoundDiag::Excluded:
+      Res.Note = "error: no match expected";
+      break;
+    case MatchFoundDiag::WrongLine:
+      Res.Note = "error: match on wrong line";
+      break;
+    case MatchFoundDiag::Discarded:
+      Res.Lead = '!'; // Not an error, but not a successful match either.
+      Res.Color = raw_ostream::CYAN;
+      Res.Note = "discard: overlaps earlier match";
+      break;
     }
-  llvm_unreachable_internal("unexpected match type");
+    break;
+  case FileCheckDiag::FCDK_MatchNoneDiag:
+    Res.Lead = 'X';
+    switch (cast<MatchNoneDiag>(Diag)->getStatus()) {
+    case MatchNoneDiag::Success:
+      break;
+    case MatchNoneDiag::InvalidPattern:
+      Res.Note = "error: match failed for invalid pattern";
+      break;
+    case MatchNoneDiag::Expected:
+      Res.Note = "error: no match found";
+      break;
+    }
+    break;
+  case FileCheckDiag::FCDK_MatchFuzzyDiag:
+    Res.Lead = '?';
+    Res.Color = raw_ostream::MAGENTA;
+    Res.Note = "possible intended match";
+    break;
+  case FileCheckDiag::FCDK_MatchCustomNoteDiag:
+    Res.Note = cast<MatchCustomNoteDiag>(Diag)->getNote();
+    if (Diag->isError())
+      Res.Note = "error: " + Res.Note;
+    break;
+  }
+  assert((!Res.FiltersAsError || !Res.Note.empty()) &&
+         "expected error diagnostic to have note");
+  return Res;
 }
 
 static void DumpInputAnnotationHelp(raw_ostream &OS) {
   OS << "The following description was requested by -dump-input=help to\n"
      << "explain the input dump printed by FileCheck.\n"
      << "\n"
      << "Related command-line options:\n"
      << "\n"
case Check::CheckMisspelled:
     return "misspelled";
   case Check::CheckNone:
     llvm_unreachable("invalid FileCheckType");
   }
   llvm_unreachable("unknown FileCheckType");
 }
 
 static void
-BuildInputAnnotations(const SourceMgr &SM, unsigned CheckFileBufferID,
+buildInputAnnotations(const SourceMgr &SM, unsigned CheckFileBufferID,
                       const std::pair<unsigned, unsigned> &ImpPatBufferIDRange,
-                      const std::vector<FileCheckDiag> &Diags,
+                      const std::vector<std::unique_ptr<FileCheckDiag>> &Diags,
                       std::vector<InputAnnotation> &Annotations,
                       unsigned &LabelWidth) {
   struct CompareSMLoc {
     bool operator()(const SMLoc &LHS, const SMLoc &RHS) const {
       return LHS.getPointer() < RHS.getPointer();
     }
   };
   // How many diagnostics does each pattern have?
   std::map<SMLoc, unsigned, CompareSMLoc> DiagCountPerPattern;
-  for (auto Diag : Diags)
-    ++DiagCountPerPattern[Diag.CheckLoc];
+  for (const std::unique_ptr<FileCheckDiag> &Diag : Diags)
+    ++DiagCountPerPattern[Diag->getMatchResultDiag().getCheckLoc()];
   // How many diagnostics have we seen so far per pattern?
   std::map<SMLoc, unsigned, CompareSMLoc> DiagIndexPerPattern;
   // How many total diagnostics have we seen so far?
   unsigned DiagIndex = 0;
   // What's the widest label?
   LabelWidth = 0;
-  for (auto DiagItr = Diags.begin(), DiagEnd = Diags.end(); DiagItr != DiagEnd;
-       ++DiagItr) {
+  // The label prefix that uniquely identifies the current MatchResultDiag and
+  // its MatchNoteDiag series.
+  std::string CurLabelPrefix;
+  for (const std::unique_ptr<FileCheckDiag> &Diag : Diags) {
     InputAnnotation A;
     A.DiagIndex = DiagIndex++;
-
-    // Build label, which uniquely identifies this check result.
-    unsigned CheckBufferID = SM.FindBufferContainingLoc(DiagItr->CheckLoc);
-    auto CheckLineAndCol =
-        SM.getLineAndColumn(DiagItr->CheckLoc, CheckBufferID);
-    llvm::raw_string_ostream Label(A.Label);
-    Label << GetCheckTypeAbbreviation(DiagItr->CheckTy) << ":";
+    if (MatchResultDiag *MRD = dyn_cast<MatchResultDiag>(Diag.get())) {
+      CurLabelPrefix.clear();
+      llvm::raw_string_ostream LabelStrm(CurLabelPrefix);
+      LabelStrm << GetCheckTypeAbbreviation(MRD->getCheckTy()) << ":";
+      unsigned CheckBufferID = SM.FindBufferContainingLoc(MRD->getCheckLoc());
       if (CheckBufferID == CheckFileBufferID)
-      Label << CheckLineAndCol.first;
+        LabelStrm
+            << SM.getLineAndColumn(MRD->getCheckLoc(), CheckBufferID).first;
       else if (ImpPatBufferIDRange.first <= CheckBufferID &&
                CheckBufferID < ImpPatBufferIDRange.second)
-      Label << "imp" << (CheckBufferID - ImpPatBufferIDRange.first + 1);
+        LabelStrm << "imp" << (CheckBufferID - ImpPatBufferIDRange.first + 1);
       else
         llvm_unreachable("expected diagnostic's check location to be either in "
                          "the check file or for an implicit pattern");
-    if (DiagCountPerPattern[DiagItr->CheckLoc] > 1)
-      Label << "'" << DiagIndexPerPattern[DiagItr->CheckLoc]++;
+    }
+
+    // Build label that uniquely identifies this FileCheckDiag.
+    llvm::raw_string_ostream LabelStrm(A.Label);
+    LabelStrm << CurLabelPrefix;
+    if (DiagCountPerPattern[Diag->getMatchResultDiag().getCheckLoc()] > 1)
+      LabelStrm
+          << "'"
+          << DiagIndexPerPattern[Diag->getMatchResultDiag().getCheckLoc()]++;
     LabelWidth = std::max((std::string::size_type)LabelWidth, A.Label.size());
 
-    A.Marker = GetMarker(DiagItr->MatchTy);
-    if (!DiagItr->Note.empty()) {
-      A.Marker.Note = DiagItr->Note;
-      // It's less confusing if notes that don't actually have ranges don't have
-      // markers.  For example, a marker for 'with "VAR" equal to "5"' would
-      // seem to indicate where "VAR" matches, but the location we actually have
-      // for the marker simply points to the start of the match/search range for
-      // the full pattern of which the substitution is potentially just one
-      // component.
-      if (DiagItr->InputStartLine == DiagItr->InputEndLine &&
-          DiagItr->InputStartCol == DiagItr->InputEndCol)
-        A.Marker.Lead = ' ';
-    }
-    if (DiagItr->MatchTy == FileCheckDiag::MatchFoundErrorNote) {
-      assert(!DiagItr->Note.empty() &&
-             "expected custom note for MatchFoundErrorNote");
-      A.Marker.Note = "error: " + A.Marker.Note;
+    // Build the input marker.
+    A.Marker = getMarker(Diag.get());
+
+    // Does this diagnostic mark text that has been successfully matched?
+    A.FoundAndExpectedMatch = false;
+    if (const MatchFoundDiag *Found = dyn_cast<MatchFoundDiag>(Diag.get())) {
+      if (Found->getStatus() == MatchFoundDiag::Success)
+        A.FoundAndExpectedMatch = true;
+    }
+
+    // If Diag has a match range, use it.  If it's a MatchNoneDiag, use its
+    // search range.  Otherwise, position it at the start of the most recent
+    // MatchResultDiag, with which it's associated.
+    unsigned InputStartLine;
+    unsigned InputStartCol;
+    unsigned InputEndLine;
+    unsigned InputEndCol;
+    if (Diag->getMatchRange().hasValue()) {
+      SMRange InputRange = Diag->getMatchRange().getValue();
+      auto InputStartLineAndCol = SM.getLineAndColumn(InputRange.Start);
+      auto InputEndLineAndCol = SM.getLineAndColumn(InputRange.End);
+      InputStartLine = InputStartLineAndCol.first;
+      InputStartCol = InputStartLineAndCol.second;
+      InputEndLine = InputEndLineAndCol.first;
+      InputEndCol = InputEndLineAndCol.second;
+    } else if (MatchNoneDiag *MND = dyn_cast<MatchNoneDiag>(Diag.get())) {
+      SMRange InputRange = MND->getSearchRange();
+      auto InputStartLineAndCol = SM.getLineAndColumn(InputRange.Start);
+      auto InputEndLineAndCol = SM.getLineAndColumn(InputRange.End);
+      InputStartLine = InputStartLineAndCol.first;
+      InputStartCol = InputStartLineAndCol.second;
+      InputEndLine = InputEndLineAndCol.first;
+      InputEndCol = InputEndLineAndCol.second;
+    } else {
+      assert(isa<MatchNoteDiag>(Diag.get()) &&
+             "expected only MatchNoteDiag to have no input range");
+      const MatchResultDiag &MRD = Diag->getMatchResultDiag();
+      SMRange InputRange;
+      if (MRD.getMatchRange().hasValue())
+        InputRange = MRD.getMatchRange().getValue();
+      else
+        InputRange = MRD.getSearchRange();
+      auto InputStartLineAndCol = SM.getLineAndColumn(InputRange.Start);
+      InputStartLine = InputEndLine = InputStartLineAndCol.first;
+      InputStartCol = InputEndCol = InputStartLineAndCol.second;
     }
-    A.FoundAndExpectedMatch =
-        DiagItr->MatchTy == FileCheckDiag::MatchFoundAndExpected;
 
     // Compute the mark location, and break annotation into multiple
     // annotations if it spans multiple lines.
     A.IsFirstLine = true;
-    A.InputLine = DiagItr->InputStartLine;
-    A.InputStartCol = DiagItr->InputStartCol;
-    if (DiagItr->InputStartLine == DiagItr->InputEndLine) {
+    A.InputLine = InputStartLine;
+    A.InputStartCol = InputStartCol;
+    if (InputStartLine == InputEndLine) {
       // Sometimes ranges are empty in order to indicate a specific point, but
       // that would mean nothing would be marked, so adjust the range to
       // include the following character.
-      A.InputEndCol =
-          std::max(DiagItr->InputStartCol + 1, DiagItr->InputEndCol);
+      A.InputEndCol = std::max(InputStartCol + 1, InputEndCol);
       Annotations.push_back(A);
     } else {
-      assert(DiagItr->InputStartLine < DiagItr->InputEndLine &&
+      assert(InputStartLine < InputEndLine &&
              "expected input range not to be inverted");
       A.InputEndCol = UINT_MAX;
       Annotations.push_back(A);
-      for (unsigned L = DiagItr->InputStartLine + 1, E = DiagItr->InputEndLine;
-           L <= E; ++L) {
+      for (unsigned L = InputStartLine + 1, E = InputEndLine; L <= E; ++L) {
         // If a range ends before the first column on a line, then it has no
         // characters on that line, so there's nothing to render.
-        if (DiagItr->InputEndCol == 1 && L == E)
+        if (InputEndCol == 1 && L == E)
           break;
         InputAnnotation B;
         B.DiagIndex = A.DiagIndex;
         B.Label = A.Label;
         B.IsFirstLine = false;
         B.InputLine = L;
         B.Marker = A.Marker;
         B.Marker.Lead = '~';
         B.Marker.Note = "";
         B.InputStartCol = 1;
         if (L != E)
           B.InputEndCol = UINT_MAX;
         else
-          B.InputEndCol = DiagItr->InputEndCol;
+          B.InputEndCol = InputEndCol;
         B.FoundAndExpectedMatch = A.FoundAndExpectedMatch;
         Annotations.push_back(B);
       }
     }
   }
 }
 
 static unsigned FindInputLineInFilter(
int main(int argc, char **argv) {
 
   SmallString<4096> InputFileBuffer;
   StringRef InputFileText = FC.CanonicalizeFile(InputFile, InputFileBuffer);
 
   SM.AddNewSourceBuffer(MemoryBuffer::getMemBuffer(
                             InputFileText, InputFile.getBufferIdentifier()),
                         SMLoc());
 
-  std::vector<FileCheckDiag> Diags;
+  FileCheckDiagList Diags;
   int ExitCode = FC.checkInput(SM, InputFileText,
                                DumpInput == DumpInputNever ? nullptr : &Diags)
                      ? EXIT_SUCCESS
                      : 1;
   if (DumpInput == DumpInputAlways ||
       (ExitCode == 1 && DumpInput == DumpInputFail)) {
     errs() << "\n"
            << "Input file: " << InputFilename << "\n"
            << "Check file: " << CheckFilename << "\n"
            << "\n"
            << "-dump-input=help explains the following input dump.\n"
            << "\n";
     std::vector<InputAnnotation> Annotations;
     unsigned LabelWidth;
-    BuildInputAnnotations(SM, CheckFileBufferID, ImpPatBufferIDRange, Diags,
-                          Annotations, LabelWidth);
+    buildInputAnnotations(SM, CheckFileBufferID, ImpPatBufferIDRange,
+                          Diags.getList(), Annotations, LabelWidth);
     DumpAnnotatedInput(errs(), Req, DumpInputFilter, DumpInputContext,
                        InputFileText, Annotations, LabelWidth);
   }
 
   return ExitCode;
 }