https://github.com/jdenny-ornl updated https://github.com/llvm/llvm-project/pull/195569
>From 1f468c7903b13911e75c8704a129e8b6e86aa18b Mon Sep 17 00:00:00 2001 From: "Joel E. Denny" <[email protected]> Date: Sun, 3 May 2026 17:58:37 -0400 Subject: [PATCH 1/3] [FileCheck][NFC] Introduce MatchResultDiag and MatchNoteDiag This patch depends on PR #???? and continues 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`). 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 series, that association is not formally modeled by `FileCheckDiag` or clearly documented, and `-dump-input` is not able to easily reason about it. This patch improves the situation by introducing two `FileCheckDiag` derived classes: `MatchResultDiag` and `MatchNoteDiag`. It extends `FileCheckDiagList` to directly associate each `MatchNoteDiag` with its `MatchResultDiag`. Thus: - `FileCheckDiagList::adjustPrevDiags` no longer has to determine that association based on directive location info. - Match result instances of `FileCheckDiag` no longer need to store a redundant custom note field, which only makes sense for notes. - Note instances of `FileCheckDiag` no longer need to store the directive type and location, which are already stored by their match result instances. - In both the FileCheck library and the `-dump-input` presentation layer, it is now clearer when a `FileCheckDiag` is a match result vs. a note. --- llvm/include/llvm/FileCheck/FileCheck.h | 153 ++++++++++++++++----- llvm/lib/FileCheck/FileCheck.cpp | 48 +++---- llvm/unittests/FileCheck/FileCheckTest.cpp | 32 +++-- llvm/utils/FileCheck/FileCheck.cpp | 101 ++++++++------ 4 files changed, 221 insertions(+), 113 deletions(-) diff --git a/llvm/include/llvm/FileCheck/FileCheck.h b/llvm/include/llvm/FileCheck/FileCheck.h index 07684e6fc6cff..b171d1048cddb 100644 --- a/llvm/include/llvm/FileCheck/FileCheck.h +++ b/llvm/include/llvm/FileCheck/FileCheck.h @@ -14,6 +14,7 @@ #define LLVM_FILECHECK_FILECHECK_H #include "llvm/ADT/StringRef.h" +#include "llvm/Support/Casting.h" #include "llvm/Support/Compiler.h" #include "llvm/Support/Regex.h" #include "llvm/Support/SMLoc.h" @@ -110,24 +111,29 @@ class FileCheckType { }; } // namespace Check -/// Summary of a FileCheck diagnostic. -struct FileCheckDiag { - /// What is the FileCheck directive for this diagnostic? - Check::FileCheckType CheckTy; - /// Where is the FileCheck directive for this diagnostic? - SMLoc CheckLoc; +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, for \c CHECK-DAG 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. +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. - /// - /// 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 { /// Indicates a good match for an expected pattern. MatchFoundAndExpected, @@ -158,39 +164,122 @@ struct FileCheckDiag { /// 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; + }; + +private: + const FileCheckDiagKind Kind; + MatchType MatchTy; + SMRange InputRange; + +public: + FileCheckDiag(FileCheckDiagKind Kind, MatchType MatchTy, SMRange InputRange) + : Kind(Kind), MatchTy(MatchTy), InputRange(InputRange) {} + /// 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; + /// 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. - unsigned InputStartLine; - unsigned InputStartCol; - unsigned InputEndLine; - unsigned InputEndCol; - /// A note to replace the one normally indicated by MatchTy, or the empty - /// string if none. - std::string Note; - LLVM_ABI FileCheckDiag(const SourceMgr &SM, - const Check::FileCheckType &CheckTy, SMLoc CheckLoc, - MatchType MatchTy, SMRange InputRange, - StringRef Note = ""); + SMRange getInputRange() const { return InputRange; } +}; + +/// Class for recording a FileCheck diagnostic that reports a match result for a +/// pattern. +class MatchResultDiag : public FileCheckDiag { +private: + Check::FileCheckType CheckTy; + SMLoc CheckLoc; + +public: + MatchResultDiag(const Check::FileCheckType &CheckTy, SMLoc CheckLoc, + MatchType MatchTy, SMRange InputRange) + : FileCheckDiag(FCDK_MatchResultDiag, MatchTy, InputRange), + CheckTy(CheckTy), CheckLoc(CheckLoc) {} + /// Is \p FCD an instance of \c MatchResultDiag? + static bool classof(const FileCheckDiag *FCD) { + return FCD->getKind() == FCDK_MatchResultDiag; + } + /// Get itself. + virtual const MatchResultDiag &getMatchResultDiag() const override { + 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; } +}; + +/// 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; + 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) {} + /// Is \p FCD an instance of \c MatchNoteDiag? + static bool classof(const FileCheckDiag *FCD) { + return FCD->getKind() == FCDK_MatchNoteDiag; + } + /// Get the note's associated \c MatchResultDiag. + virtual const MatchResultDiag &getMatchResultDiag() const override { + return *MRD; + } + /// Set the note's associated \c MatchResultDiag. + void setMatchResultDiag(MatchResultDiag *MRDNew) { + 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; } }; /// 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. - template <typename... ArgTys> void emplace_back(const ArgTys &...Args) { - DiagList.emplace_back(std::make_unique<FileCheckDiag>(Args...)); + /// 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 recent consecutive diagnostics of the same \c CheckLoc to have - /// \c MatchTy. + /// 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) { - SMLoc CheckLoc = (*DiagList.rbegin())->CheckLoc; + assert(CurMatchResultDiag && "expected previous MatchResultDiag"); for (auto I = DiagList.rbegin(), E = DiagList.rend(); - I != E && (*I)->CheckLoc == CheckLoc; ++I) - (*I)->MatchTy = MatchTy; + I != E && &**I != CurMatchResultDiag; ++I) + (*I)->adjustMatchType(MatchTy); + CurMatchResultDiag->adjustMatchType(MatchTy); } /// 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 7fd672b978e36..b493eb67fa173 100644 --- a/llvm/lib/FileCheck/FileCheck.cpp +++ b/llvm/lib/FileCheck/FileCheck.cpp @@ -1285,8 +1285,8 @@ void Pattern::printSubstitutions(const SourceMgr &SM, StringRef Buffer, // 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<MatchNoteDiag>( + MatchTy, SMRange(Range.Start, Range.Start), OS.str()); else SM.PrintMessage(Range.Start, SourceMgr::DK_Note, OS.str()); } @@ -1340,7 +1340,7 @@ void Pattern::printVariableDefs(const SourceMgr &SM, 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<MatchNoteDiag>(MatchTy, VC.Range, OS.str()); else SM.PrintMessage(VC.Range.Start, SourceMgr::DK_Note, OS.str(), VC.Range); } @@ -1399,8 +1399,7 @@ void Pattern::printFuzzyMatch(const SourceMgr &SM, StringRef Buffer, if (Best && Best != StringRef::npos && BestQuality < 50) { SMRange MatchRange = buildMatchRange(Buffer, Best, 0); if (Diags) - Diags->emplace_back(SM, getCheckTy(), getLoc(), FileCheckDiag::MatchFuzzy, - MatchRange); + Diags->emplace<MatchNoteDiag>(FileCheckDiag::MatchFuzzy, MatchRange); SM.PrintMessage(MatchRange.Start, SourceMgr::DK_Note, "possible intended match here"); @@ -1507,18 +1506,7 @@ StringRef FileCheck::CanonicalizeFile(MemoryBuffer &MB, 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() {} static bool IsPartOfWord(char c) { return (isAlnum(c) || c == '-' || c == '_'); @@ -2045,7 +2033,7 @@ static Error printMatch(bool ExpectedMatch, const SourceMgr &SM, SMRange MatchRange = buildMatchRange(Buffer, MatchResult.TheMatch->Pos, MatchResult.TheMatch->Len); if (Diags) { - Diags->emplace_back(SM, Pat.getCheckTy(), Loc, MatchTy, MatchRange); + Diags->emplace<MatchResultDiag>(Pat.getCheckTy(), Loc, MatchTy, MatchRange); Pat.printSubstitutions(SM, Buffer, MatchRange, MatchTy, Diags); Pat.printVariableDefs(SM, MatchTy, Diags); } @@ -2077,9 +2065,9 @@ static Error printMatch(bool ExpectedMatch, const SourceMgr &SM, [&](const ErrorDiagnostic &E) { E.log(errs()); if (Diags) { - Diags->emplace_back(SM, Pat.getCheckTy(), Loc, - FileCheckDiag::MatchFoundErrorNote, - E.getRange(), E.getMessage().str()); + Diags->emplace<MatchNoteDiag>( + FileCheckDiag::MatchFoundErrorNote, E.getRange(), + E.getMessage().str()); } }); return ErrorReported::reportedOrSuccess(HasError); @@ -2131,11 +2119,11 @@ static Error printNoMatch(bool ExpectedMatch, const SourceMgr &SM, // diagnostic is all we have to anchor them. SMRange SearchRange = buildSearchRange(Buffer); if (Diags) { - Diags->emplace_back(SM, Pat.getCheckTy(), Loc, MatchTy, SearchRange); + Diags->emplace<MatchResultDiag>(Pat.getCheckTy(), Loc, MatchTy, + SearchRange); SMRange NoteRange = SMRange(SearchRange.Start, SearchRange.Start); for (StringRef ErrorMsg : ErrorMsgs) - Diags->emplace_back(SM, Pat.getCheckTy(), Loc, MatchTy, NoteRange, - ErrorMsg); + Diags->emplace<MatchNoteDiag>(MatchTy, NoteRange, ErrorMsg); Pat.printSubstitutions(SM, Buffer, SearchRange, MatchTy, Diags); } if (!PrintDiag) { @@ -2268,9 +2256,9 @@ size_t FileCheckString::Check(const SourceMgr &SM, StringRef Buffer, if (Req.Verbose) { Diags->adjustPrevDiags(FileCheckDiag::MatchFoundButWrongLine); } else { - Diags->emplace_back(SM, Pat.getCheckTy(), Loc, - FileCheckDiag::MatchFoundButWrongLine, - buildMatchRange(MatchBuffer, MatchPos, MatchLen)); + Diags->emplace<MatchResultDiag>( + Pat.getCheckTy(), Loc, FileCheckDiag::MatchFoundButWrongLine, + buildMatchRange(MatchBuffer, MatchPos, MatchLen)); } } return StringRef::npos; @@ -2283,9 +2271,9 @@ size_t FileCheckString::Check(const SourceMgr &SM, StringRef Buffer, if (Req.Verbose) { Diags->adjustPrevDiags(FileCheckDiag::MatchFoundButWrongLine); } else { - Diags->emplace_back(SM, Pat.getCheckTy(), Loc, - FileCheckDiag::MatchFoundButWrongLine, - buildMatchRange(MatchBuffer, MatchPos, MatchLen)); + Diags->emplace<MatchResultDiag>( + Pat.getCheckTy(), Loc, FileCheckDiag::MatchFoundButWrongLine, + buildMatchRange(MatchBuffer, MatchPos, MatchLen)); } } return StringRef::npos; diff --git a/llvm/unittests/FileCheck/FileCheckTest.cpp b/llvm/unittests/FileCheck/FileCheckTest.cpp index cff958a114c63..9282296b4c540 100644 --- a/llvm/unittests/FileCheck/FileCheckTest.cpp +++ b/llvm/unittests/FileCheck/FileCheckTest.cpp @@ -918,8 +918,21 @@ class PatternTester { FileCheckDiagList &Diags) { P.printVariableDefs(SM, MatchTy, &Diags); } + + SourceMgr &getSourceMgr() { return SM; } }; +#define EXPECT_SM_RANGE(SM, Range, StartLineExpected, StartColExpected, \ + EndLineExpected, EndColExpected) \ + do { \ + auto StartActual = SM.getLineAndColumn(Range.Start); \ + auto EndActual = SM.getLineAndColumn(Range.End); \ + EXPECT_EQ(StartActual.first, StartLineExpected); \ + EXPECT_EQ(StartActual.second, StartColExpected); \ + EXPECT_EQ(EndActual.first, EndLineExpected); \ + EXPECT_EQ(EndActual.second, EndColExpected); \ + } while (0) + TEST_F(FileCheckTest, ParseNumericSubstitutionBlock) { PatternTester Tester; @@ -1639,22 +1652,21 @@ TEST_F(FileCheckTest, FileCheckContext) { 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()); FileCheckDiagList Diags; Tester.printVariableDefs(FileCheckDiag::MatchFoundAndExpected, Diags); EXPECT_EQ(Diags.getList().size(), 2ul); + SmallVector<MatchNoteDiag, 2> Notes; for (const std::unique_ptr<FileCheckDiag> &Diag : Diags.getList()) { - EXPECT_EQ(Diag->CheckTy, Check::CheckPlain); - EXPECT_EQ(Diag->MatchTy, FileCheckDiag::MatchFoundAndExpected); - EXPECT_EQ(Diag->InputStartLine, 1u); - EXPECT_EQ(Diag->InputEndLine, 1u); + EXPECT_EQ(Diag->getKind(), FileCheckDiag::FCDK_MatchNoteDiag); + EXPECT_EQ(Diag->getMatchType(), FileCheckDiag::MatchFoundAndExpected); + Notes.push_back(cast<MatchNoteDiag>(*Diag)); } - EXPECT_EQ(Diags.getList()[0]->InputStartCol, 1u); - EXPECT_EQ(Diags.getList()[0]->InputEndCol, 7u); - EXPECT_EQ(Diags.getList()[1]->InputStartCol, 8u); - EXPECT_EQ(Diags.getList()[1]->InputEndCol, 9u); - EXPECT_EQ(Diags.getList()[0]->Note, "captured var \"STRVAR\""); - EXPECT_EQ(Diags.getList()[1]->Note, "captured var \"NUMVAR\""); + 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\""); } } // namespace diff --git a/llvm/utils/FileCheck/FileCheck.cpp b/llvm/utils/FileCheck/FileCheck.cpp index 6cf762bda68d0..7b29238844e4f 100644 --- a/llvm/utils/FileCheck/FileCheck.cpp +++ b/llvm/utils/FileCheck/FileCheck.cpp @@ -390,76 +390,95 @@ buildInputAnnotations(const SourceMgr &SM, unsigned CheckFileBufferID, // How many diagnostics does each pattern have? std::map<SMLoc, unsigned, CompareSMLoc> DiagCountPerPattern; for (const std::unique_ptr<FileCheckDiag> &Diag : Diags) - ++DiagCountPerPattern[Diag->CheckLoc]; + ++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; + // 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++; + 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) + LabelStrm + << SM.getLineAndColumn(MRD->getCheckLoc(), CheckBufferID).first; + else if (ImpPatBufferIDRange.first <= CheckBufferID && + CheckBufferID < ImpPatBufferIDRange.second) + 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"); + } - // Build label, which uniquely identifies this check result. - unsigned CheckBufferID = SM.FindBufferContainingLoc(Diag->CheckLoc); - auto CheckLineAndCol = SM.getLineAndColumn(Diag->CheckLoc, CheckBufferID); - llvm::raw_string_ostream Label(A.Label); - Label << GetCheckTypeAbbreviation(Diag->CheckTy) << ":"; - if (CheckBufferID == CheckFileBufferID) - Label << CheckLineAndCol.first; - else if (ImpPatBufferIDRange.first <= CheckBufferID && - CheckBufferID < ImpPatBufferIDRange.second) - Label << "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[Diag->CheckLoc] > 1) - Label << "'" << DiagIndexPerPattern[Diag->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(Diag->MatchTy); - if (!Diag->Note.empty()) { - A.Marker.Note = Diag->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 (Diag->InputStartLine == Diag->InputEndLine && - Diag->InputStartCol == Diag->InputEndCol) - A.Marker.Lead = ' '; + 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 = ' '; + } } - if (Diag->MatchTy == FileCheckDiag::MatchFoundErrorNote) { - assert(!Diag->Note.empty() && - "expected custom note for MatchFoundErrorNote"); + if (Diag->getMatchType() == FileCheckDiag::MatchFoundErrorNote) { + assert(CustomNote && "expected custom note for MatchFoundErrorNote"); A.Marker.Note = "error: " + A.Marker.Note; } A.FoundAndExpectedMatch = - Diag->MatchTy == FileCheckDiag::MatchFoundAndExpected; + Diag->getMatchType() == FileCheckDiag::MatchFoundAndExpected; // Compute the mark 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 = Diag->InputStartLine; - A.InputStartCol = Diag->InputStartCol; - if (Diag->InputStartLine == Diag->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(Diag->InputStartCol + 1, Diag->InputEndCol); + A.InputEndCol = std::max(InputStartCol + 1, InputEndCol); Annotations.push_back(A); } else { - assert(Diag->InputStartLine < Diag->InputEndLine && + assert(InputStartLine < InputEndLine && "expected input range not to be inverted"); A.InputEndCol = UINT_MAX; Annotations.push_back(A); - for (unsigned L = Diag->InputStartLine + 1, E = Diag->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 (Diag->InputEndCol == 1 && L == E) + if (InputEndCol == 1 && L == E) break; InputAnnotation B; B.DiagIndex = A.DiagIndex; @@ -473,7 +492,7 @@ buildInputAnnotations(const SourceMgr &SM, unsigned CheckFileBufferID, if (L != E) B.InputEndCol = UINT_MAX; else - B.InputEndCol = Diag->InputEndCol; + B.InputEndCol = InputEndCol; B.FoundAndExpectedMatch = A.FoundAndExpectedMatch; Annotations.push_back(B); } >From 425edb5ff7f2012b3ecdb5db9f82418232af1545 Mon Sep 17 00:00:00 2001 From: "Joel E. Denny" <[email protected]> Date: Tue, 5 May 2026 09:19:46 -0400 Subject: [PATCH 2/3] Drop virtual if override --- llvm/include/llvm/FileCheck/FileCheck.h | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/llvm/include/llvm/FileCheck/FileCheck.h b/llvm/include/llvm/FileCheck/FileCheck.h index c2cb73cf746f4..9a95e76a98cd8 100644 --- a/llvm/include/llvm/FileCheck/FileCheck.h +++ b/llvm/include/llvm/FileCheck/FileCheck.h @@ -208,9 +208,7 @@ class MatchResultDiag : public FileCheckDiag { return FCD->getKind() == FCDK_MatchResultDiag; } /// Get itself. - virtual const MatchResultDiag &getMatchResultDiag() const override { - return *this; - } + const MatchResultDiag &getMatchResultDiag() const override { 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? @@ -234,9 +232,7 @@ class MatchNoteDiag : public FileCheckDiag { return FCD->getKind() == FCDK_MatchNoteDiag; } /// Get the note's associated \c MatchResultDiag. - virtual const MatchResultDiag &getMatchResultDiag() const override { - return *MRD; - } + const MatchResultDiag &getMatchResultDiag() const override { return *MRD; } /// Set the note's associated \c MatchResultDiag. void setMatchResultDiag(MatchResultDiag *MRDNew) { assert(!MRD && "expected setMatchResultDiag to be called only once"); >From b6fa0615c4a4ba3dd905e81fec8184d39916c7a6 Mon Sep 17 00:00:00 2001 From: "Joel E. Denny" <[email protected]> Date: Tue, 5 May 2026 09:38:33 -0400 Subject: [PATCH 3/3] Drop redundant init --- llvm/utils/FileCheck/FileCheck.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/utils/FileCheck/FileCheck.cpp b/llvm/utils/FileCheck/FileCheck.cpp index 6492b32861f47..0a8d0e1d10572 100644 --- a/llvm/utils/FileCheck/FileCheck.cpp +++ b/llvm/utils/FileCheck/FileCheck.cpp @@ -429,7 +429,7 @@ buildInputAnnotations(const SourceMgr &SM, unsigned CheckFileBufferID, LabelWidth = std::max((std::string::size_type)LabelWidth, A.Label.size()); A.Marker = GetMarker(Diag.getMatchType()); - std::optional<StringRef> CustomNote = std::nullopt; + std::optional<StringRef> CustomNote; if (const MatchNoteDiag *NoteDiag = dyn_cast<MatchNoteDiag>(&Diag)) { CustomNote = NoteDiag->getCustomNote(); if (CustomNote) { _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
