https://github.com/jdenny-ornl updated https://github.com/llvm/llvm-project/pull/195571
>From 25f8dbdb87750286f2a003334de9ec7b723f00d8 Mon Sep 17 00:00:00 2001 From: "Joel E. Denny" <[email protected]> Date: Sun, 3 May 2026 18:01:23 -0400 Subject: [PATCH 1/3] [FileCheck][NFC] Complete FileCheckDiag class hierarchy This patch depends on PR #???? and finishes its effort to decouple the `-dump-input` presentation layer (in `llvm/utils/FileCheck/FileCheck.cpp`) and the FileCheck library's diagnostic emission (in `llvm/lib/FileCheck/FileCheck.cpp`). The focus of this patch is finally eliminating `enum MatchType` and completing the `FileCheckDiag` class hierarchy. That enables the following improvements. Replace MatchTy and InputRange ============================== `-dump-input` needs some means to determine (1) whether some notes should be filtered in by `-dump-input-filter=error`, and (2) where to place some notes in the input dump. Without this patch series, the FileCheck library provides that information by copying the following from a match result to any note that does not 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. However, this copied information is redundant as it is already part of the associated match result, and semantically it does not really apply to the note itself. Furthermore, for a note that does not have a match location (e.g., the FileCheck library does not 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. This interaction is too subtle, even in the case of just `-dump-input`. Moreover, it is easy to imagine alternative diagnostic presentation layers where it would be unhelpful and thus more misleading. For example, imagine (this is just a thought experiment, and I have no plans for this) an HTML-based presentation where notes appear in pop-up windows when clicking on match results. A copied error status and location is then not relevant for notes. Association with a match result still is relevant. This patch removes the `MatchTy` and `InputRange` fields from `FieldCheckDiag`. It replaces `MatchTy` with `Status` fields in just the `MatchResultDiag` hierarchy. To replace `InputRange`, it exposes a match range as a `std::optional`, and it adds a search range to `MatchResultDiag` (used now for `MatchNoneDiag` and, in the future, `-dump-input` will present search ranges upon any error). 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. Clean up getMarker ================== Because of the now complete `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. --- llvm/include/llvm/FileCheck/FileCheck.h | 304 ++++++++++++++++----- llvm/lib/FileCheck/FileCheck.cpp | 82 +++--- llvm/lib/FileCheck/FileCheckImpl.h | 4 +- llvm/unittests/FileCheck/FileCheckTest.cpp | 27 +- llvm/utils/FileCheck/FileCheck.cpp | 158 ++++++----- 5 files changed, 376 insertions(+), 199 deletions(-) diff --git a/llvm/include/llvm/FileCheck/FileCheck.h b/llvm/include/llvm/FileCheck/FileCheck.h index b171d1048cddb..ea66ca3cb0617 100644 --- a/llvm/include/llvm/FileCheck/FileCheck.h +++ b/llvm/include/llvm/FileCheck/FileCheck.h @@ -124,56 +124,31 @@ class MatchResultDiag; /// - \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 { FCDK_MatchResultDiag, FCDK_MatchNoteDiag }; - - /// 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. - enum MatchType { - /// Indicates a good match for an expected pattern. - MatchFoundAndExpected, - /// Indicates a match for an excluded pattern. - MatchFoundButExcluded, - /// 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, - /// 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, - /// 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, + 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; - MatchType MatchTy; - SMRange InputRange; public: - FileCheckDiag(FileCheckDiagKind Kind, MatchType MatchTy, SMRange InputRange) - : Kind(Kind), MatchTy(MatchTy), InputRange(InputRange) {} + 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? @@ -181,30 +156,41 @@ class FileCheckDiag { /// 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; - /// Adjust the match type. - void adjustMatchType(MatchType MatchTy) { this->MatchTy = MatchTy; } - /// Get the match type. - MatchType getMatchType() const { return MatchTy; } - /// The search range if MatchTy starts with MatchNone, or the match range - /// otherwise. - SMRange getInputRange() const { return InputRange; } + /// Does this diagnostic reveal a new 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 std::nullopt if the diagnostic + /// has no such input range. + virtual std::optional<SMRange> getMatchRange() const = 0; }; -/// Class for recording a FileCheck diagnostic that reports a match result for a -/// pattern. +/// Abstract base class for recording a FileCheck diagnostic that reports a +/// match result for a pattern. class MatchResultDiag : public FileCheckDiag { private: Check::FileCheckType CheckTy; SMLoc CheckLoc; + SMRange SearchRange; public: - MatchResultDiag(const Check::FileCheckType &CheckTy, SMLoc CheckLoc, - MatchType MatchTy, SMRange InputRange) - : FileCheckDiag(FCDK_MatchResultDiag, MatchTy, InputRange), - CheckTy(CheckTy), CheckLoc(CheckLoc) {} + 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) { - return FCD->getKind() == FCDK_MatchResultDiag; + FileCheckDiagKind FCDK = FCD->getKind(); + return FCDK_MatchResultDiag_First <= FCDK && + FCDK <= FCDK_MatchResultDiag_Last; } /// Get itself. virtual const MatchResultDiag &getMatchResultDiag() const override { @@ -214,23 +200,122 @@ class MatchResultDiag : public FileCheckDiag { 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; } }; -/// Class for recording a FileCheck diagnostic that provides an additional note -/// (possibly an additional error) about the most recent \c MatchResultDiag. +/// \c MatchResultDiag for a pattern that matched the input. +class MatchFoundDiag : public MatchResultDiag { +public: + enum StatusTy { + /// Indicates a good match for an expected pattern. + 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 (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 override { + 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 std::nullopt. + virtual std::optional<SMRange> getMatchRange() const override { + 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. + Success, + /// Indicates no match due to an expected or excluded pattern that has + /// 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). In some cases, it + /// follows good matches (because multiple matches are expected) or + /// 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 override { return Status != Success; } + /// Does the lack of a match indicate a success? If not, why not? + StatusTy getStatus() const { return Status; } + /// Return \c std::nullopt. + virtual std::optional<SMRange> getMatchRange() const override { + return std::nullopt; + } +}; + +/// Abstract base class for recording a FileCheck diagnostic that provides an +/// additional note (possibly a new error) about the most recent +/// \c MatchResultDiag. class MatchNoteDiag : public FileCheckDiag { private: MatchResultDiag *MRD; - std::optional<std::string> CustomNote; public: - MatchNoteDiag(MatchType MatchTy, SMRange InputRange, - std::optional<StringRef> CustomNote = std::nullopt) - : FileCheckDiag(FCDK_MatchNoteDiag, MatchTy, InputRange), MRD(nullptr), - CustomNote(CustomNote) {} + 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) { - return FCD->getKind() == FCDK_MatchNoteDiag; + 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 override { @@ -241,9 +326,82 @@ class MatchNoteDiag : public FileCheckDiag { assert(!MRD && "expected setMatchResultDiag to be called only once"); MRD = MRDNew; } - /// A note to replace the one normally indicated by the match type, or the - /// empty string if none. - const std::optional<std::string> &getCustomNote() const { return CustomNote; } +}; + +/// \c MatchNoteDiag for a fuzzy match that serves as a suggestion for the next +/// intended match for an expected pattern with too few or no good matches. +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 override { return false; } + /// Return an input range (never \c std::nullopt) starting and ending at the + /// match start. The actual match end is not computed. + virtual std::optional<SMRange> getMatchRange() const override { + 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; + bool AddsError; + std::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 a \a new 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 might indicate a match to either an expected pattern + /// (success) or an excluded pattern (error), and \c Note might be "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 override { return AddsError; } + /// Return the match range described by the note, or \c std::nullopt if none. + virtual std::optional<SMRange> getMatchRange() const override { + return MatchRange; + } }; /// A \c FileCheckDiag series emitted by the FileCheck library. @@ -272,14 +430,10 @@ class FileCheckDiagList { return; Note->setMatchResultDiag(CurMatchResultDiag); } - /// Adjust the most recent \c MatchResultDiag, which must exist, and every - /// \c MatchResultNote after it to have the match type \c MatchTy. - void adjustPrevDiags(FileCheckDiag::MatchType MatchTy) { - assert(CurMatchResultDiag && "expected previous MatchResultDiag"); - for (auto I = DiagList.rbegin(), E = DiagList.rend(); - I != E && &**I != CurMatchResultDiag; ++I) - (*I)->adjustMatchType(MatchTy); - CurMatchResultDiag->adjustMatchType(MatchTy); + /// 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 { diff --git a/llvm/lib/FileCheck/FileCheck.cpp b/llvm/lib/FileCheck/FileCheck.cpp index b493eb67fa173..4d3ac87b9c724 100644 --- a/llvm/lib/FileCheck/FileCheck.cpp +++ b/llvm/lib/FileCheck/FileCheck.cpp @@ -1260,7 +1260,6 @@ unsigned Pattern::computeMatchDistance(StringRef Buffer) const { void Pattern::printSubstitutions(const SourceMgr &SM, StringRef Buffer, SMRange Range, - FileCheckDiag::MatchType MatchTy, FileCheckDiagList *Diags) const { // Print what we know about substitutions. if (!Substitutions.empty()) { @@ -1280,13 +1279,13 @@ void Pattern::printSubstitutions(const SourceMgr &SM, StringRef Buffer, OS.write_escaped(Substitution->getFromString()) << "\" equal to "; OS << *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<MatchNoteDiag>( - MatchTy, SMRange(Range.Start, Range.Start), OS.str()); + Diags->emplace<MatchCustomNoteDiag>(OS.str()); else SM.PrintMessage(Range.Start, SourceMgr::DK_Note, OS.str()); } @@ -1294,7 +1293,6 @@ void Pattern::printSubstitutions(const SourceMgr &SM, StringRef Buffer, } void Pattern::printVariableDefs(const SourceMgr &SM, - FileCheckDiag::MatchType MatchTy, FileCheckDiagList *Diags) const { if (VariableDefs.empty() && NumericVariableDefs.empty()) return; @@ -1340,7 +1338,7 @@ void Pattern::printVariableDefs(const SourceMgr &SM, raw_svector_ostream OS(Msg); OS << "captured var \"" << VC.Name << "\""; if (Diags) - Diags->emplace<MatchNoteDiag>(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); } @@ -1397,10 +1395,10 @@ void Pattern::printFuzzyMatch(const SourceMgr &SM, StringRef Buffer, // reasonable and not equal to what we showed in the "scanning from here" // line. if (Best && Best != StringRef::npos && BestQuality < 50) { - SMRange MatchRange = buildMatchRange(Buffer, Best, 0); + SMLoc MatchStart = SMLoc::getFromPointer(Buffer.data() + Best); if (Diags) - Diags->emplace<MatchNoteDiag>(FileCheckDiag::MatchFuzzy, MatchRange); - SM.PrintMessage(MatchRange.Start, SourceMgr::DK_Note, + 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 @@ -1507,6 +1505,8 @@ StringRef FileCheck::CanonicalizeFile(MemoryBuffer &MB, } FileCheckDiag::~FileCheckDiag() {} +MatchResultDiag::~MatchResultDiag() {} +MatchNoteDiag::~MatchNoteDiag() {} static bool IsPartOfWord(char c) { return (isAlnum(c) || c == '-' || c == '_'); @@ -2027,15 +2027,16 @@ static Error printMatch(bool ExpectedMatch, const SourceMgr &SM, } // Add "found" diagnostic, substitutions, and variable definitions to Diags. - FileCheckDiag::MatchType MatchTy = ExpectedMatch - ? FileCheckDiag::MatchFoundAndExpected - : FileCheckDiag::MatchFoundButExcluded; + MatchFoundDiag::StatusTy Status = + ExpectedMatch ? MatchFoundDiag::Success : MatchFoundDiag::Excluded; SMRange MatchRange = buildMatchRange(Buffer, MatchResult.TheMatch->Pos, MatchResult.TheMatch->Len); + SMRange SearchRange = buildSearchRange(Buffer); if (Diags) { - Diags->emplace<MatchResultDiag>(Pat.getCheckTy(), Loc, MatchTy, MatchRange); - 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"); @@ -2055,8 +2056,8 @@ static Error printMatch(bool ExpectedMatch, const SourceMgr &SM, {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 @@ -2065,9 +2066,9 @@ static Error printMatch(bool ExpectedMatch, const SourceMgr &SM, [&](const ErrorDiagnostic &E) { E.log(errs()); if (Diags) { - Diags->emplace<MatchNoteDiag>( - FileCheckDiag::MatchFoundErrorNote, E.getRange(), - E.getMessage().str()); + Diags->emplace<MatchCustomNoteDiag>(E.getRange(), + E.getMessage().str(), + /*AddsError=*/true); } }); return ErrorReported::reportedOrSuccess(HasError); @@ -2083,15 +2084,14 @@ static Error printNoMatch(bool ExpectedMatch, const SourceMgr &SM, // 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()); @@ -2119,12 +2119,10 @@ static Error printNoMatch(bool ExpectedMatch, const SourceMgr &SM, // diagnostic is all we have to anchor them. SMRange SearchRange = buildSearchRange(Buffer); if (Diags) { - Diags->emplace<MatchResultDiag>(Pat.getCheckTy(), Loc, MatchTy, - SearchRange); - SMRange NoteRange = SMRange(SearchRange.Start, SearchRange.Start); + Diags->emplace<MatchNoneDiag>(Pat.getCheckTy(), Loc, Status, SearchRange); for (StringRef ErrorMsg : ErrorMsgs) - Diags->emplace<MatchNoteDiag>(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"); @@ -2150,7 +2148,7 @@ static Error printNoMatch(bool ExpectedMatch, const SourceMgr &SM, // 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); @@ -2254,11 +2252,12 @@ size_t FileCheckString::Check(const SourceMgr &SM, StringRef Buffer, if (CheckNext(SM, SkippedRegion)) { if (Diags) { if (Req.Verbose) { - Diags->adjustPrevDiags(FileCheckDiag::MatchFoundButWrongLine); + Diags->adjustPrevMatchFoundDiag(MatchFoundDiag::WrongLine); } else { - Diags->emplace<MatchResultDiag>( - Pat.getCheckTy(), Loc, FileCheckDiag::MatchFoundButWrongLine, - buildMatchRange(MatchBuffer, MatchPos, MatchLen)); + Diags->emplace<MatchFoundDiag>( + Pat.getCheckTy(), Loc, MatchFoundDiag::WrongLine, + buildMatchRange(MatchBuffer, MatchPos, MatchLen), + buildSearchRange(MatchBuffer)); } } return StringRef::npos; @@ -2269,11 +2268,12 @@ size_t FileCheckString::Check(const SourceMgr &SM, StringRef Buffer, if (CheckSame(SM, SkippedRegion)) { if (Diags) { if (Req.Verbose) { - Diags->adjustPrevDiags(FileCheckDiag::MatchFoundButWrongLine); + Diags->adjustPrevMatchFoundDiag(MatchFoundDiag::WrongLine); } else { - Diags->emplace<MatchResultDiag>( - Pat.getCheckTy(), Loc, FileCheckDiag::MatchFoundButWrongLine, - buildMatchRange(MatchBuffer, MatchPos, MatchLen)); + Diags->emplace<MatchFoundDiag>( + Pat.getCheckTy(), Loc, MatchFoundDiag::WrongLine, + buildMatchRange(MatchBuffer, MatchPos, MatchLen), + buildSearchRange(MatchBuffer)); } } return StringRef::npos; @@ -2464,7 +2464,7 @@ FileCheckString::CheckDag(const SourceMgr &SM, StringRef Buffer, // we're gathering them for a different rendering, but we always print // other diagnostics. if (Diags) { - Diags->adjustPrevDiags(FileCheckDiag::MatchFoundButDiscarded); + Diags->adjustPrevMatchFoundDiag(MatchFoundDiag::Discarded); } else { SMLoc OldStart = SMLoc::getFromPointer(Buffer.data() + MI->Pos); SMLoc OldEnd = SMLoc::getFromPointer(Buffer.data() + MI->End); diff --git a/llvm/lib/FileCheck/FileCheckImpl.h b/llvm/lib/FileCheck/FileCheckImpl.h index 547c93f2acca5..5393650020a30 100644 --- a/llvm/lib/FileCheck/FileCheckImpl.h +++ b/llvm/lib/FileCheck/FileCheckImpl.h @@ -733,8 +733,7 @@ class Pattern { const SourceMgr &SM) const; /// Prints the value of successful substitutions. void printSubstitutions(const SourceMgr &SM, StringRef Buffer, - SMRange MatchRange, FileCheckDiag::MatchType MatchTy, - FileCheckDiagList *Diags) const; + SMRange MatchRange, FileCheckDiagList *Diags) const; void printFuzzyMatch(const SourceMgr &SM, StringRef Buffer, FileCheckDiagList *Diags) const; @@ -742,7 +741,6 @@ class Pattern { return !(Substitutions.empty() && VariableDefs.empty()); } LLVM_ABI_FOR_TEST void printVariableDefs(const SourceMgr &SM, - FileCheckDiag::MatchType MatchTy, FileCheckDiagList *Diags) const; Check::FileCheckType getCheckTy() const { return CheckTy; } diff --git a/llvm/unittests/FileCheck/FileCheckTest.cpp b/llvm/unittests/FileCheck/FileCheckTest.cpp index 9282296b4c540..909f52babbb13 100644 --- a/llvm/unittests/FileCheck/FileCheckTest.cpp +++ b/llvm/unittests/FileCheck/FileCheckTest.cpp @@ -914,17 +914,18 @@ class PatternTester { return Res.TheMatch->Pos; } - void printVariableDefs(FileCheckDiag::MatchType MatchTy, - FileCheckDiagList &Diags) { - P.printVariableDefs(SM, MatchTy, &Diags); + void printVariableDefs(FileCheckDiagList &Diags) { + P.printVariableDefs(SM, &Diags); } SourceMgr &getSourceMgr() { return SM; } }; -#define EXPECT_SM_RANGE(SM, Range, StartLineExpected, StartColExpected, \ +#define EXPECT_SM_RANGE(SM, RangeOpt, StartLineExpected, StartColExpected, \ EndLineExpected, EndColExpected) \ do { \ + EXPECT_TRUE(RangeOpt->isValid()); \ + SMRange Range = *RangeOpt; \ auto StartActual = SM.getLineAndColumn(Range.Start); \ auto EndActual = SM.getLineAndColumn(Range.End); \ EXPECT_EQ(StartActual.first, StartLineExpected); \ @@ -1656,17 +1657,17 @@ TEST_F(FileCheckTest, CapturedVarDiags) { ASSERT_FALSE(Tester.parsePattern("[[STRVAR:[a-z]+]] [[#NUMVAR:@LINE]]")); EXPECT_THAT_EXPECTED(Tester.match("foobar 2"), Succeeded()); FileCheckDiagList Diags; - Tester.printVariableDefs(FileCheckDiag::MatchFoundAndExpected, Diags); + Tester.printVariableDefs(Diags); EXPECT_EQ(Diags.getList().size(), 2ul); - SmallVector<MatchNoteDiag, 2> Notes; + SmallVector<MatchCustomNoteDiag, 2> Notes; for (const std::unique_ptr<FileCheckDiag> &Diag : Diags.getList()) { - EXPECT_EQ(Diag->getKind(), FileCheckDiag::FCDK_MatchNoteDiag); - EXPECT_EQ(Diag->getMatchType(), FileCheckDiag::MatchFoundAndExpected); - Notes.push_back(cast<MatchNoteDiag>(*Diag)); + EXPECT_EQ(Diag->getKind(), FileCheckDiag::FCDK_MatchCustomNoteDiag); + EXPECT_FALSE(Diag->isError()); + Notes.push_back(cast<MatchCustomNoteDiag>(*Diag)); } - EXPECT_SM_RANGE(SM, Notes[0].getInputRange(), 1u, 1u, 1u, 7u); - EXPECT_SM_RANGE(SM, Notes[1].getInputRange(), 1u, 8u, 1u, 9u); - EXPECT_EQ(Notes[0].getCustomNote(), "captured var \"STRVAR\""); - EXPECT_EQ(Notes[1].getCustomNote(), "captured var \"NUMVAR\""); + EXPECT_SM_RANGE(SM, Notes[0].getMatchRange(), 1u, 1u, 1u, 7u); + EXPECT_SM_RANGE(SM, Notes[1].getMatchRange(), 1u, 8u, 1u, 9u); + EXPECT_EQ(Notes[0].getNote(), "captured var \"STRVAR\""); + EXPECT_EQ(Notes[1].getNote(), "captured var \"NUMVAR\""); } } // namespace diff --git a/llvm/utils/FileCheck/FileCheck.cpp b/llvm/utils/FileCheck/FileCheck.cpp index 7b29238844e4f..459e13cd2933d 100644 --- a/llvm/utils/FileCheck/FileCheck.cpp +++ b/llvm/utils/FileCheck/FileCheck.cpp @@ -192,47 +192,71 @@ struct MarkerStyle { std::string Note; /// Does this marker indicate inclusion by -dump-input-filter=error? bool FiltersAsError; - MarkerStyle() = default; - 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() ? ' ' : IsError ? '!' : '^'; + Res.Color = IsError ? raw_ostream::RED : raw_ostream::GREEN; + Res.FiltersAsError = IsError; + + // Add Note. Override the default Lead and Color for some diagnostic kinds. + switch (Diag->getKind()) { + case FileCheckDiag::FCDK_MatchFoundDiag: + switch (cast<MatchFoundDiag>(Diag)->getStatus()) { + case MatchFoundDiag::Success: + break; + case MatchFoundDiag::Excluded: + Res.Note = "no match expected"; + break; + case MatchFoundDiag::WrongLine: + Res.Note = "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; + } + break; + case FileCheckDiag::FCDK_MatchNoneDiag: + Res.Lead = 'X'; + switch (cast<MatchNoneDiag>(Diag)->getStatus()) { + case MatchNoneDiag::Success: + break; + case MatchNoneDiag::InvalidPattern: + Res.Note = "match failed for invalid pattern"; + break; + case MatchNoneDiag::Expected: + Res.Note = "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(); + break; } - llvm_unreachable_internal("unexpected match type"); + if (Diag->isError()) { + assert(!Res.Note.empty() && "expected error diagnostic to have note"); + Res.Note = "error: " + Res.Note; + } + return Res; } static void DumpInputAnnotationHelp(raw_ostream &OS) { @@ -428,39 +452,39 @@ buildInputAnnotations(const SourceMgr &SM, unsigned CheckFileBufferID, << DiagIndexPerPattern[Diag->getMatchResultDiag().getCheckLoc()]++; LabelWidth = std::max((std::string::size_type)LabelWidth, A.Label.size()); - A.Marker = GetMarker(Diag->getMatchType()); - std::optional<StringRef> CustomNote = std::nullopt; - if (const MatchNoteDiag *NoteDiag = dyn_cast<MatchNoteDiag>(&*Diag)) { - CustomNote = NoteDiag->getCustomNote(); - if (CustomNote) { - A.Marker.Note = *CustomNote; - // 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. - SMRange InputRange = Diag->getInputRange(); - if (InputRange.Start == InputRange.End) - A.Marker.Lead = ' '; - } + // 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->getMatchType() == FileCheckDiag::MatchFoundErrorNote) { - assert(CustomNote && "expected custom note for MatchFoundErrorNote"); - A.Marker.Note = "error: " + A.Marker.Note; + + // If Diag has a match range, position the marker there. If it is a + // MatchNoneDiag, position the marker at its search range. Otherwise, + // position the marker at the start of the most recent MatchResultDiag with + // which it is associated. + SMRange InputRange; + if (Diag->getMatchRange()) { + InputRange = *Diag->getMatchRange(); + } else if (MatchNoneDiag *MND = dyn_cast<MatchNoneDiag>(Diag.get())) { + InputRange = MND->getSearchRange(); + } else { + assert(isa<MatchNoteDiag>(Diag.get()) && + "expected only MatchNoteDiag to have no input range"); + const MatchResultDiag &MRD = Diag->getMatchResultDiag(); + InputRange = MRD.getMatchRange() ? *MRD.getMatchRange() + : MRD.getSearchRange(); + InputRange.End = InputRange.Start; } - A.FoundAndExpectedMatch = - Diag->getMatchType() == FileCheckDiag::MatchFoundAndExpected; + auto [InputStartLine, InputStartCol] = + SM.getLineAndColumn(InputRange.Start); + auto [InputEndLine, InputEndCol] = SM.getLineAndColumn(InputRange.End); - // Compute the mark location, and break annotation into multiple + // Compute the marker location, and break annotation into multiple // annotations if it spans multiple lines. - SMRange InputRange = Diag->getInputRange(); - auto InputStartLineAndCol = SM.getLineAndColumn(InputRange.Start); - auto InputEndLineAndCol = SM.getLineAndColumn(InputRange.End); - unsigned InputStartLine = InputStartLineAndCol.first; - unsigned InputStartCol = InputStartLineAndCol.second; - unsigned InputEndLine = InputEndLineAndCol.first; - unsigned InputEndCol = InputEndLineAndCol.second; A.IsFirstLine = true; A.InputLine = InputStartLine; A.InputStartCol = InputStartCol; >From 2a338aec06f68a34f26c33b1e161c2321e33918f Mon Sep 17 00:00:00 2001 From: "Joel E. Denny" <[email protected]> Date: Sun, 3 May 2026 21:27:44 -0400 Subject: [PATCH 2/3] Apply clang-format --- llvm/utils/FileCheck/FileCheck.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/llvm/utils/FileCheck/FileCheck.cpp b/llvm/utils/FileCheck/FileCheck.cpp index 459e13cd2933d..3088777fa846f 100644 --- a/llvm/utils/FileCheck/FileCheck.cpp +++ b/llvm/utils/FileCheck/FileCheck.cpp @@ -475,8 +475,8 @@ buildInputAnnotations(const SourceMgr &SM, unsigned CheckFileBufferID, assert(isa<MatchNoteDiag>(Diag.get()) && "expected only MatchNoteDiag to have no input range"); const MatchResultDiag &MRD = Diag->getMatchResultDiag(); - InputRange = MRD.getMatchRange() ? *MRD.getMatchRange() - : MRD.getSearchRange(); + InputRange = + MRD.getMatchRange() ? *MRD.getMatchRange() : MRD.getSearchRange(); InputRange.End = InputRange.Start; } auto [InputStartLine, InputStartCol] = >From 82bbf0b1971f98440d99added554c36e0307aee5 Mon Sep 17 00:00:00 2001 From: "Joel E. Denny" <[email protected]> Date: Tue, 5 May 2026 09:45:00 -0400 Subject: [PATCH 3/3] Drop virtual if override --- llvm/include/llvm/FileCheck/FileCheck.h | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/llvm/include/llvm/FileCheck/FileCheck.h b/llvm/include/llvm/FileCheck/FileCheck.h index 802a31babc769..de76fa789caee 100644 --- a/llvm/include/llvm/FileCheck/FileCheck.h +++ b/llvm/include/llvm/FileCheck/FileCheck.h @@ -235,7 +235,7 @@ class MatchFoundDiag : public MatchResultDiag { /// /// 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 override { + bool isError() const override { return Status != Success && Status != Discarded; } /// Was this a successful match? If not, why not? @@ -257,9 +257,7 @@ class MatchFoundDiag : public MatchResultDiag { Status = S; } /// Return the match's input range, never \c std::nullopt. - virtual std::optional<SMRange> getMatchRange() const override { - return MatchRange; - } + std::optional<SMRange> getMatchRange() const override { return MatchRange; } }; /// \c MatchResultDiag for a pattern that did not match the input. @@ -291,13 +289,11 @@ class MatchNoneDiag : public MatchResultDiag { return FCD->getKind() == FCDK_MatchNoneDiag; } /// Does the lack of match represent an error? - virtual bool isError() const override { return Status != Success; } + bool isError() const override { return Status != Success; } /// Does the lack of a match indicate a success? If not, why not? StatusTy getStatus() const { return Status; } /// Return \c std::nullopt. - virtual std::optional<SMRange> getMatchRange() const override { - return std::nullopt; - } + std::optional<SMRange> getMatchRange() const override { return std::nullopt; } }; /// Abstract base class for recording a FileCheck diagnostic that provides an @@ -340,10 +336,10 @@ class MatchFuzzyDiag : public MatchNoteDiag { } /// Always false. A fuzzy match is not an error even though it is performed /// due to an error. - virtual bool isError() const override { return false; } + bool isError() const override { return false; } /// Return an input range (never \c std::nullopt) starting and ending at the /// match start. The actual match end is not computed. - virtual std::optional<SMRange> getMatchRange() const override { + std::optional<SMRange> getMatchRange() const override { return SMRange(MatchStart, MatchStart); } }; @@ -394,11 +390,9 @@ class MatchCustomNoteDiag : public MatchNoteDiag { /// /// For details, see the \c MatchCustomNoteDiag::MatchCustomNoteDiag comments /// for its \c AddsError parameter. - virtual bool isError() const override { return AddsError; } + bool isError() const override { return AddsError; } /// Return the match range described by the note, or \c std::nullopt if none. - virtual std::optional<SMRange> getMatchRange() const override { - return MatchRange; - } + std::optional<SMRange> getMatchRange() const override { return MatchRange; } }; /// A \c FileCheckDiag series emitted by the FileCheck library. _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
