[llvm-branch-commits] [llvm] [mlir] [Support] Fix offset parsing and buffer caching in SourceMgrDiagnosticVerifier (PR #227567)
Alexander Richardson via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Sun Oct 18 12:42:19 PDT 2026
https://github.com/arichardson updated https://github.com/llvm/llvm-project/pull/227567
>From a7f40d199db89c6024013dd0edbc4f3f8a6894f0 Mon Sep 17 00:00:00 2001
From: Alex Richardson <alexrichardson at google.com>
Date: Tue, 29 Sep 2026 22:49:12 -0700
Subject: [PATCH 1/2] [Support] Fix offset parsing and buffer caching in
SourceMgrDiagnosticVerifier
Fix three bugs inherited from the original MLIR implementation:
- Parse `@+N` / `@-N` line offsets explicitly in base 10 so leading zeros
like `@+010` are not interpreted as octal, and reject negative offsets
whose magnitude exceeds the current line number instead of silently
underflowing `unsigned`.
- Make `computeExpectedDiags` idempotent when called multiple times on the
same buffer so existing matched diagnostics are not wiped out.
- Rescan all buffers currently registered in `SourceMgr` inside `verify()`
so buffers that never emitted a diagnostic still have their `expected-*`
directives checked.
This commit was created with the help of AI tools
---
.../Support/SourceMgrDiagnosticVerifier.cpp | 45 +++++++++++++++----
1 file changed, 36 insertions(+), 9 deletions(-)
diff --git a/llvm/lib/Support/SourceMgrDiagnosticVerifier.cpp b/llvm/lib/Support/SourceMgrDiagnosticVerifier.cpp
index 44124c25c7d21..832f54332a955 100644
--- a/llvm/lib/Support/SourceMgrDiagnosticVerifier.cpp
+++ b/llvm/lib/Support/SourceMgrDiagnosticVerifier.cpp
@@ -88,6 +88,10 @@ SourceMgrDiagnosticVerifier::computeExpectedDiags(raw_ostream &OS,
// If the buffer is invalid, return an empty list.
if (!Buf)
return {};
+ // Idempotent: a buffer that's already been scanned is returned as-is,
+ // rather than being rescanned and appending duplicate entries.
+ if (auto MaybeDiags = getExpectedDiags(Buf->getBufferIdentifier()))
+ return *MaybeDiags;
auto &ExpectedDiags = ExpectedDiagsPerFile[Buf->getBufferIdentifier()];
// The number of the last line that did not correlate to a designator.
@@ -141,12 +145,30 @@ SourceMgrDiagnosticVerifier::computeExpectedDiags(raw_ostream &OS,
// Get the integer value without the @ and +/- prefix.
if (OffsetMatch[0] == '+' || OffsetMatch[0] == '-') {
int Offset;
- OffsetMatch.drop_front().getAsInteger(0, Offset);
+ // The regex only ever captures decimal digits here, so parse as
+ // decimal explicitly: a leading zero (e.g. '@+08') would otherwise
+ // be mis-parsed as octal by getAsInteger's radix auto-detection
+ // (and '08' isn't valid octal, so that already-broken corner case
+ // would silently discard the offset instead of reporting an error).
+ if (OffsetMatch.drop_front().getAsInteger(10, Offset)) {
+ OK = false;
+ Record.emitError(OS, Mgr, "invalid line offset '" + OffsetMatch + "'");
+ continue;
+ }
- if (OffsetMatch.front() == '+')
+ if (OffsetMatch.front() == '+') {
Record.LineNo += Offset;
- else
+ } else if (static_cast<unsigned>(Offset) >= Record.LineNo) {
+ // Line numbers are 1-based, so an offset that would take LineNo to
+ // 0 or below is out of range.
+ OK = false;
+ Record.emitError(OS, Mgr,
+ "line offset '" + OffsetMatch +
+ "' is before the start of the file");
+ continue;
+ } else {
Record.LineNo -= Offset;
+ }
} else if (OffsetMatch.consume_front("unknown")) {
// This is matching unknown locations.
Record.FileLoc = SMLoc();
@@ -189,12 +211,8 @@ SourceMgrDiagnosticVerifier::process(raw_ostream &OS, SourceMgr &Mgr,
if (HasLoc) {
// If the buffer couldn't be resolved, `Diags` stays empty: a diagnostic
// with a location in an unknown file can never match anything.
- if (Buf) {
- if (auto MaybeDiags = getExpectedDiags(Buf->getBufferIdentifier()))
- Diags = *MaybeDiags;
- else
- Diags = computeExpectedDiags(OS, Mgr, Buf);
- }
+ if (Buf)
+ Diags = computeExpectedDiags(OS, Mgr, Buf);
} else {
Diags = ExpectedUnknownLocDiags;
}
@@ -237,6 +255,15 @@ SourceMgrDiagnosticVerifier::process(raw_ostream &OS, SourceMgr &Mgr,
}
bool SourceMgrDiagnosticVerifier::verify(raw_ostream &OS, SourceMgr &Mgr) {
+ // Scan any buffers that process() never had a reason to look at (e.g. an
+ // included file where the diagnostic its 'expected-*' comment names never
+ // actually fired): otherwise that expectation would never be recorded, and
+ // so never reported as missing below, letting verification silently
+ // pass when it shouldn't. computeExpectedDiags is idempotent, so
+ // re-scanning an already-known buffer here is harmless.
+ for (unsigned I = 0, E = Mgr.getNumBuffers(); I != E; ++I)
+ (void)computeExpectedDiags(OS, Mgr, Mgr.getMemoryBuffer(I + 1));
+
// Verify that all expected errors were seen.
auto CheckExpectedDiags = [&](ExpectedDiag &Diag) {
if (!Diag.Matched) {
>From d3732b1dc3abd26d56360c1d06dd1dc52fc92041 Mon Sep 17 00:00:00 2001
From: Alex Richardson <alexrichardson at google.com>
Date: Sun, 18 Oct 2026 12:40:41 -0700
Subject: [PATCH 2/2] [nspr] update
---
.../Support/SourceMgrDiagnosticVerifier.h | 141 ++++++------
.../Support/SourceMgrDiagnosticVerifier.cpp | 200 ++++++++++--------
llvm/unittests/Support/CMakeLists.txt | 1 +
.../SourceMgrDiagnosticVerifierTest.cpp | 174 +++++++++++++++
.../secondary/llvm/unittests/Support/BUILD.gn | 1 +
mlir/lib/IR/Diagnostics.cpp | 15 +-
6 files changed, 361 insertions(+), 171 deletions(-)
create mode 100644 llvm/unittests/Support/SourceMgrDiagnosticVerifierTest.cpp
diff --git a/llvm/include/llvm/Support/SourceMgrDiagnosticVerifier.h b/llvm/include/llvm/Support/SourceMgrDiagnosticVerifier.h
index 675c1a781bdd4..0c1dc380065c1 100644
--- a/llvm/include/llvm/Support/SourceMgrDiagnosticVerifier.h
+++ b/llvm/include/llvm/Support/SourceMgrDiagnosticVerifier.h
@@ -17,25 +17,77 @@
#define LLVM_SUPPORT_SOURCEMGRDIAGNOSTICVERIFIER_H
#include <optional>
+#include <string>
+#include <vector>
#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/SmallVector.h"
-#include "llvm/ADT/StringMap.h"
#include "llvm/Support/Compiler.h"
#include "llvm/Support/Regex.h"
#include "llvm/Support/SourceMgr.h"
+#include "llvm/Support/raw_ostream.h"
namespace llvm {
class MemoryBuffer;
-class raw_ostream;
/// Scans SourceMgr source buffers for 'expected-<kind>' comments (e.g.
/// 'expected-error {{message}}') and verifies that the diagnostics reported
/// to it via \c process match them, and that every expected diagnostic is
-/// matched at least once.
+/// matched at least once. At most one directive may appear on a single line,
+/// and nothing (other than whitespace) may follow the closing '}}' on that
+/// line.
class LLVM_ABI SourceMgrDiagnosticVerifier {
public:
+ /// The result of matching a single actual diagnostic against the expected
+ /// diagnostics recorded for its location.
+ enum class MatchResult {
+ /// The diagnostic matched an expected diagnostic (kind and text).
+ Matched,
+ /// The diagnostic's text matched an expected diagnostic, but its kind
+ /// didn't; a message about this has already been printed.
+ NearMiss,
+ /// The diagnostic did not match any expected diagnostic. The caller is
+ /// responsible for reporting it, if desired.
+ Unexpected,
+ /// The diagnostic did not match, but \p ReportUnexpected was false, so
+ /// nothing was printed and nothing needs to be done.
+ Ignored,
+ };
+
+ /// \param Mgr The \c SourceMgr whose buffers are scanned for expected
+ /// diagnostics.
+ /// \param OS The stream to which verifier diagnostics (near misses,
+ /// unmatched expected diagnostics, malformed directives) are printed.
+ explicit SourceMgrDiagnosticVerifier(const SourceMgr &Mgr,
+ raw_ostream &OS = errs());
+
+ /// Matches a single actual diagnostic against the expected diagnostics
+ /// recorded for \p Buf / \p LineNo, scanning \p Buf first if it hasn't been
+ /// scanned yet. If \p HasLoc is false, the diagnostic has no location and is
+ /// matched against the '@unknown' list instead (\p Buf / \p LineNo are
+ /// ignored). If \p HasLoc is true but \p Buf is null (e.g. the diagnostic's
+ /// file isn't a known buffer), the diagnostic is matched against an empty
+ /// list, i.e. it can never match and is always unexpected. On a near miss,
+ /// prints a message to \c OS. \p ReportUnexpected controls whether near
+ /// misses / unexpected diagnostics are reported at all.
+ MatchResult process(SourceMgr::DiagKind Kind, bool HasLoc,
+ const MemoryBuffer *Buf, unsigned LineNo,
+ StringRef Message, bool ReportUnexpected = true);
+
+ /// Convenience overload for diagnostics already packaged as an
+ /// \c SMDiagnostic (e.g. from a \c SourceMgr diagnostic handler). Resolves
+ /// the source buffer and line from \p Diag, and if the diagnostic is
+ /// unexpected and \p ReportUnexpected is true, prints \p Diag to \c OS.
+ MatchResult process(const SMDiagnostic &Diag, bool ReportUnexpected = true);
+
+ /// Reports (to \c OS) any expected diagnostic that was never matched by a
+ /// call to \p process. Returns whether verification succeeded overall, i.e.
+ /// no diagnostic mismatches were recorded either here or by \p process.
+ bool verify();
+
+private:
/// A single diagnostic expected via an 'expected-<kind>' comment.
struct ExpectedDiag {
ExpectedDiag(SourceMgr::DiagKind Kind, unsigned LineNo, SMLoc FileLoc,
@@ -45,16 +97,6 @@ class LLVM_ABI SourceMgrDiagnosticVerifier {
/// Returns true if this diagnostic matches the given message.
LLVM_ABI bool match(StringRef Str) const;
- /// Computes the regex matcher for a '-re' diagnostic's substring.
- /// Returns false and prints a message through \p Mgr on error.
- LLVM_ABI bool computeRegex(raw_ostream &OS, SourceMgr &Mgr);
-
- /// Prints \p Msg at this diagnostic's location and returns false, for
- /// use as `return emitError(...);` in functions that report failure via
- /// a bool return.
- LLVM_ABI bool emitError(raw_ostream &OS, SourceMgr &Mgr,
- const Twine &Msg) const;
-
/// The severity of the diagnostic expected.
SourceMgr::DiagKind Kind;
/// The line number the expected diagnostic should be on.
@@ -70,68 +112,33 @@ class LLVM_ABI SourceMgrDiagnosticVerifier {
std::optional<Regex> SubstringRegex;
};
- /// The result of matching a single actual diagnostic against the expected
- /// diagnostics recorded for its location.
- enum class MatchResult {
- /// The diagnostic matched an expected diagnostic (kind and text).
- Matched,
- /// The diagnostic's text matched an expected diagnostic, but its kind
- /// didn't; a message about this has already been printed.
- NearMiss,
- /// The diagnostic did not match any expected diagnostic. The caller is
- /// responsible for reporting it, if desired.
- Unexpected,
- /// The diagnostic did not match, but \p ReportUnexpected was false, so
- /// nothing was printed and nothing needs to be done.
- Ignored,
- };
+ /// Prints \p Diag to \c OS directly, bypassing \c Mgr.getDiagHandler() so
+ /// that a verifier installed as \c Mgr's diagnostic handler does not
+ /// re-enter itself when printing its own diagnostics.
+ void printDiag(const SMDiagnostic &Diag) const;
+
+ /// Records a verification failure (`OK = false`), prints \p Msg at \p Diag's
+ /// location to \c OS, and returns false.
+ bool emitError(const ExpectedDiag &Diag, const Twine &Msg);
- SourceMgrDiagnosticVerifier() = default;
+ /// Computes the regex matcher for a '-re' diagnostic's substring. Returns
+ /// false and prints a message to \c OS on error.
+ bool computeRegex(ExpectedDiag &Diag);
/// Computes and caches the list of expected diagnostics for \p Buf, if not
/// already cached. Returns the (mutable) cached list.
- MutableArrayRef<ExpectedDiag> computeExpectedDiags(raw_ostream &OS,
- SourceMgr &Mgr,
- const MemoryBuffer *Buf);
-
- /// Returns the cached expected diagnostics for the buffer named \p BufName,
- /// or std::nullopt if \p computeExpectedDiags hasn't been called for it.
- std::optional<MutableArrayRef<ExpectedDiag>>
- getExpectedDiags(StringRef BufName);
+ MutableArrayRef<ExpectedDiag> computeExpectedDiags(const MemoryBuffer *Buf);
- /// Returns the expected diagnostics with an '@unknown' location.
- MutableArrayRef<ExpectedDiag> getExpectedUnknownLocDiags() {
- return ExpectedUnknownLocDiags;
- }
+ const SourceMgr &Mgr;
+ raw_ostream &OS;
- /// Matches a single actual diagnostic against the expected diagnostics
- /// recorded for \p Buf / \p LineNo, computing them first via \p
- /// computeExpectedDiags if they haven't been already. If \p HasLoc is
- /// false, the diagnostic has no location and is matched against the
- /// '@unknown' list instead (\p Buf / \p LineNo are ignored). If \p HasLoc
- /// is true but \p Buf is null (e.g. the diagnostic's file isn't a known
- /// buffer), the diagnostic is matched against an empty list, i.e. it can
- /// never match and is always unexpected. On a near miss, prints a message
- /// through \p Mgr. \p ReportUnexpected controls whether near misses /
- /// unexpected diagnostics are reported at all.
- MatchResult process(raw_ostream &OS, SourceMgr &Mgr, SourceMgr::DiagKind Kind,
- bool HasLoc, const MemoryBuffer *Buf, unsigned LineNo,
- StringRef Message, bool ReportUnexpected = true);
-
- /// Reports (through \p Mgr) any expected diagnostic that was never matched
- /// by a call to \p process. Returns whether verification succeeded overall,
- /// i.e. no diagnostic mismatches were recorded either here or by \p
- /// process.
- bool verify(raw_ostream &OS, SourceMgr &Mgr);
-
-private:
/// Regex used to recognize 'expected-<kind>' comments.
- Regex Expected{"expected-(error|note|remark|warning)(-re)? "
- "*(@([+-][0-9]+|above|below|unknown))? *{{(.*)}}$"};
+ Regex Expected{"expected-(error|note|remark|warning)(-re)?"
+ " *(@([+-][0-9]+|above|below|unknown))? *{{(.*)}}$"};
- /// The expected diagnostics for each buffer that has been scanned so far,
- /// keyed by buffer identifier (i.e. file name).
- StringMap<SmallVector<ExpectedDiag, 2>> ExpectedDiagsPerFile;
+ /// The expected diagnostics for each buffer that has been scanned so far.
+ DenseMap<const MemoryBuffer *, SmallVector<ExpectedDiag, 2>>
+ ExpectedDiagsPerBuffer;
/// The expected diagnostics with an '@unknown' location.
SmallVector<ExpectedDiag, 2> ExpectedUnknownLocDiags;
diff --git a/llvm/lib/Support/SourceMgrDiagnosticVerifier.cpp b/llvm/lib/Support/SourceMgrDiagnosticVerifier.cpp
index 832f54332a955..71f4b0e1c1c1b 100644
--- a/llvm/lib/Support/SourceMgrDiagnosticVerifier.cpp
+++ b/llvm/lib/Support/SourceMgrDiagnosticVerifier.cpp
@@ -8,11 +8,16 @@
#include "llvm/Support/SourceMgrDiagnosticVerifier.h"
+#include "llvm/ADT/StringSwitch.h"
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/raw_ostream.h"
using namespace llvm;
+SourceMgrDiagnosticVerifier::SourceMgrDiagnosticVerifier(const SourceMgr &Mgr,
+ raw_ostream &OS)
+ : Mgr(Mgr), OS(OS) {}
+
/// Given a diagnostic kind, return a human readable string for it.
static StringRef getDiagKindStr(SourceMgr::DiagKind Kind) {
switch (Kind) {
@@ -28,15 +33,22 @@ static StringRef getDiagKindStr(SourceMgr::DiagKind Kind) {
llvm_unreachable("Unknown SourceMgr::DiagKind");
}
-bool SourceMgrDiagnosticVerifier::ExpectedDiag::emitError(
- raw_ostream &OS, SourceMgr &Mgr, const Twine &Msg) const {
- if (FileLoc.isValid()) {
- SMRange Range(FileLoc, SMLoc::getFromPointer(FileLoc.getPointer() +
- Substring.size()));
- Mgr.PrintMessage(OS, FileLoc, SourceMgr::DK_Error, Msg, Range);
- } else {
- Mgr.PrintMessage(OS, FileLoc, SourceMgr::DK_Error, Msg);
+void SourceMgrDiagnosticVerifier::printDiag(const SMDiagnostic &Diag) const {
+ Mgr.printIncludeStackForDiagnostic(Diag.getLoc(), OS);
+ Diag.print(nullptr, OS);
+}
+
+bool SourceMgrDiagnosticVerifier::emitError(const ExpectedDiag &Diag,
+ const Twine &Msg) {
+ OK = false;
+ SMRange Range;
+ ArrayRef<SMRange> Ranges;
+ if (Diag.FileLoc.isValid()) {
+ Range =
+ SMRange(Diag.FileLoc, SMLoc::getFromPointer(Diag.Substring.end() + 2));
+ Ranges = Range;
}
+ printDiag(Mgr.GetMessage(Diag.FileLoc, SourceMgr::DK_Error, Msg, Ranges));
return false;
}
@@ -48,11 +60,10 @@ bool SourceMgrDiagnosticVerifier::ExpectedDiag::match(StringRef Str) const {
return Str.contains(Substring);
}
-bool SourceMgrDiagnosticVerifier::ExpectedDiag::computeRegex(raw_ostream &OS,
- SourceMgr &Mgr) {
+bool SourceMgrDiagnosticVerifier::computeRegex(ExpectedDiag &Diag) {
std::string RegexStr;
raw_string_ostream RegexOS(RegexStr);
- StringRef StrToProcess = Substring;
+ StringRef StrToProcess = Diag.Substring;
while (!StrToProcess.empty()) {
// Find the next regex block.
size_t RegexIt = StrToProcess.find("{{");
@@ -66,33 +77,30 @@ bool SourceMgrDiagnosticVerifier::ExpectedDiag::computeRegex(raw_ostream &OS,
// Find the end of the regex block.
size_t RegexEndIt = StrToProcess.find("}}");
if (RegexEndIt == StringRef::npos)
- return emitError(OS, Mgr, "found start of regex with no end '}}'");
+ return emitError(Diag, "found start of regex with no end '}}'");
StringRef RegexBlock = StrToProcess.take_front(RegexEndIt);
// Validate that the regex is actually valid.
std::string RegexError;
if (!Regex(RegexBlock).isValid(RegexError))
- return emitError(OS, Mgr, "invalid regex: " + RegexError);
+ return emitError(Diag, "invalid regex: " + RegexError);
RegexOS << '(' << RegexBlock << ')';
StrToProcess = StrToProcess.drop_front(RegexEndIt + 2);
}
- SubstringRegex = Regex(RegexStr);
+ Diag.SubstringRegex = Regex(RegexStr);
return true;
}
MutableArrayRef<SourceMgrDiagnosticVerifier::ExpectedDiag>
-SourceMgrDiagnosticVerifier::computeExpectedDiags(raw_ostream &OS,
- SourceMgr &Mgr,
- const MemoryBuffer *Buf) {
+SourceMgrDiagnosticVerifier::computeExpectedDiags(const MemoryBuffer *Buf) {
// If the buffer is invalid, return an empty list.
if (!Buf)
return {};
- // Idempotent: a buffer that's already been scanned is returned as-is,
- // rather than being rescanned and appending duplicate entries.
- if (auto MaybeDiags = getExpectedDiags(Buf->getBufferIdentifier()))
- return *MaybeDiags;
- auto &ExpectedDiags = ExpectedDiagsPerFile[Buf->getBufferIdentifier()];
+ auto [It, Inserted] = ExpectedDiagsPerBuffer.try_emplace(Buf);
+ if (!Inserted)
+ return It->second;
+ auto &ExpectedDiags = It->second;
// The number of the last line that did not correlate to a designator.
unsigned LastNonDesignatorLine = 0;
@@ -104,7 +112,7 @@ SourceMgrDiagnosticVerifier::computeExpectedDiags(raw_ostream &OS,
SmallVector<StringRef, 100> Lines;
Buf->getBuffer().split(Lines, '\n');
for (unsigned LineNo = 0, E = Lines.size(); LineNo < E; ++LineNo) {
- SmallVector<StringRef, 4> Matches;
+ SmallVector<StringRef, 10> Matches;
if (!Expected.match(Lines[LineNo].rtrim(), &Matches)) {
// Check for designators that apply to this line.
if (!DesignatorsForNextLine.empty()) {
@@ -119,24 +127,16 @@ SourceMgrDiagnosticVerifier::computeExpectedDiags(raw_ostream &OS,
// Point to the start of expected-*.
SMLoc ExpectedStart = SMLoc::getFromPointer(Matches[0].data());
- SourceMgr::DiagKind Kind;
- if (Matches[1] == "error")
- Kind = SourceMgr::DK_Error;
- else if (Matches[1] == "warning")
- Kind = SourceMgr::DK_Warning;
- else if (Matches[1] == "remark")
- Kind = SourceMgr::DK_Remark;
- else {
- assert(Matches[1] == "note");
- Kind = SourceMgr::DK_Note;
- }
+ SourceMgr::DiagKind Kind = StringSwitch<SourceMgr::DiagKind>(Matches[1])
+ .Case("error", SourceMgr::DK_Error)
+ .Case("warning", SourceMgr::DK_Warning)
+ .Case("remark", SourceMgr::DK_Remark)
+ .Case("note", SourceMgr::DK_Note);
ExpectedDiag Record(Kind, LineNo + 1, ExpectedStart, Matches[5]);
// Check to see if this is a regex match, i.e. it includes the `-re`.
- if (!Matches[2].empty() && !Record.computeRegex(OS, Mgr)) {
- OK = false;
+ if (!Matches[2].empty() && !computeRegex(Record))
continue;
- }
StringRef OffsetMatch = Matches[3];
if (!OffsetMatch.empty()) {
@@ -144,27 +144,26 @@ SourceMgrDiagnosticVerifier::computeExpectedDiags(raw_ostream &OS,
// Get the integer value without the @ and +/- prefix.
if (OffsetMatch[0] == '+' || OffsetMatch[0] == '-') {
- int Offset;
- // The regex only ever captures decimal digits here, so parse as
- // decimal explicitly: a leading zero (e.g. '@+08') would otherwise
- // be mis-parsed as octal by getAsInteger's radix auto-detection
- // (and '08' isn't valid octal, so that already-broken corner case
- // would silently discard the offset instead of reporting an error).
+ unsigned Offset;
+ // Parse as decimal explicitly so leading zeros (e.g. '@+08') are not
+ // treated as octal by getAsInteger's radix auto-detection.
if (OffsetMatch.drop_front().getAsInteger(10, Offset)) {
- OK = false;
- Record.emitError(OS, Mgr, "invalid line offset '" + OffsetMatch + "'");
+ emitError(Record, "invalid line offset '" + OffsetMatch + "'");
continue;
}
if (OffsetMatch.front() == '+') {
+ if (Offset > E - Record.LineNo) {
+ emitError(Record, "line offset '" + OffsetMatch +
+ "' is past the end of the file");
+ continue;
+ }
Record.LineNo += Offset;
- } else if (static_cast<unsigned>(Offset) >= Record.LineNo) {
- // Line numbers are 1-based, so an offset that would take LineNo to
- // 0 or below is out of range.
- OK = false;
- Record.emitError(OS, Mgr,
- "line offset '" + OffsetMatch +
- "' is before the start of the file");
+ } else if (Offset > Record.LineNo) {
+ // Allow LineNo == 0 (Offset == Record.LineNo), which MLIR uses for
+ // diagnostics on the implicit top-level module.
+ emitError(Record, "line offset '" + OffsetMatch +
+ "' is before the start of the file");
continue;
} else {
Record.LineNo -= Offset;
@@ -194,17 +193,8 @@ SourceMgrDiagnosticVerifier::computeExpectedDiags(raw_ostream &OS,
return ExpectedDiags;
}
-std::optional<MutableArrayRef<SourceMgrDiagnosticVerifier::ExpectedDiag>>
-SourceMgrDiagnosticVerifier::getExpectedDiags(StringRef BufName) {
- auto ExpectedDiags = ExpectedDiagsPerFile.find(BufName);
- if (ExpectedDiags != ExpectedDiagsPerFile.end())
- return MutableArrayRef<ExpectedDiag>(ExpectedDiags->second);
- return std::nullopt;
-}
-
SourceMgrDiagnosticVerifier::MatchResult
-SourceMgrDiagnosticVerifier::process(raw_ostream &OS, SourceMgr &Mgr,
- SourceMgr::DiagKind Kind, bool HasLoc,
+SourceMgrDiagnosticVerifier::process(SourceMgr::DiagKind Kind, bool HasLoc,
const MemoryBuffer *Buf, unsigned LineNo,
StringRef Message, bool ReportUnexpected) {
MutableArrayRef<ExpectedDiag> Diags;
@@ -212,31 +202,40 @@ SourceMgrDiagnosticVerifier::process(raw_ostream &OS, SourceMgr &Mgr,
// If the buffer couldn't be resolved, `Diags` stays empty: a diagnostic
// with a location in an unknown file can never match anything.
if (Buf)
- Diags = computeExpectedDiags(OS, Mgr, Buf);
+ Diags = computeExpectedDiags(Buf);
} else {
+ for (unsigned I = 0, E = Mgr.getNumBuffers(); I != E; ++I)
+ (void)computeExpectedDiags(Mgr.getMemoryBuffer(I + 1));
Diags = ExpectedUnknownLocDiags;
}
- // Search for a matching expected diagnostic.
- // If we find something that is close then emit a more specific error.
+ // Search for a matching expected diagnostic. Prefer an unmatched entry so
+ // that multiple expected-* directives on the same line with identical or
+ // overlapping substrings each get a chance to match before an already-matched
+ // entry is reused.
+ ExpectedDiag *AlreadyMatched = nullptr;
ExpectedDiag *NearMiss = nullptr;
- // If this was an expected error, remember that we saw it and return.
for (auto &E : Diags) {
// File line must match (unless it's an unknown location).
if (HasLoc && E.LineNo != LineNo)
continue;
- if (E.match(Message)) {
- if (E.Kind == Kind) {
+ if (!E.match(Message))
+ continue;
+ if (E.Kind == Kind) {
+ if (!E.Matched) {
E.Matched = true;
return MatchResult::Matched;
}
-
+ AlreadyMatched = &E;
+ } else if (!E.Matched) {
// If this only differs based on the diagnostic kind, then consider it
// to be a near miss.
NearMiss = &E;
}
}
+ if (AlreadyMatched)
+ return MatchResult::Matched;
if (!ReportUnexpected)
return MatchResult::Ignored;
@@ -245,39 +244,52 @@ SourceMgrDiagnosticVerifier::process(raw_ostream &OS, SourceMgr &Mgr,
// Otherwise, emit an error for the near miss.
if (NearMiss) {
- Mgr.PrintMessage(OS, NearMiss->FileLoc, SourceMgr::DK_Error,
- "'" + getDiagKindStr(Kind) +
- "' diagnostic emitted when expecting a '" +
- getDiagKindStr(NearMiss->Kind) + "'");
+ printDiag(Mgr.GetMessage(NearMiss->FileLoc, SourceMgr::DK_Error,
+ "'" + getDiagKindStr(Kind) +
+ "' diagnostic emitted when expecting a '" +
+ getDiagKindStr(NearMiss->Kind) + "'"));
return MatchResult::NearMiss;
}
return MatchResult::Unexpected;
}
-bool SourceMgrDiagnosticVerifier::verify(raw_ostream &OS, SourceMgr &Mgr) {
- // Scan any buffers that process() never had a reason to look at (e.g. an
- // included file where the diagnostic its 'expected-*' comment names never
- // actually fired): otherwise that expectation would never be recorded, and
- // so never reported as missing below, letting verification silently
- // pass when it shouldn't. computeExpectedDiags is idempotent, so
- // re-scanning an already-known buffer here is harmless.
- for (unsigned I = 0, E = Mgr.getNumBuffers(); I != E; ++I)
- (void)computeExpectedDiags(OS, Mgr, Mgr.getMemoryBuffer(I + 1));
+SourceMgrDiagnosticVerifier::MatchResult
+SourceMgrDiagnosticVerifier::process(const SMDiagnostic &Diag,
+ bool ReportUnexpected) {
+ bool HasLoc = Diag.getLoc().isValid();
+ const MemoryBuffer *Buf = nullptr;
+ if (HasLoc) {
+ if (unsigned ID = Mgr.FindBufferContainingLoc(Diag.getLoc()))
+ Buf = Mgr.getMemoryBuffer(ID);
+ }
+ MatchResult Result =
+ process(Diag.getKind(), HasLoc, Buf,
+ HasLoc ? static_cast<unsigned>(Diag.getLineNo()) : 0,
+ Diag.getMessage(), ReportUnexpected);
+ if (Result == MatchResult::Unexpected)
+ printDiag(Diag);
+ return Result;
+}
- // Verify that all expected errors were seen.
- auto CheckExpectedDiags = [&](ExpectedDiag &Diag) {
- if (!Diag.Matched) {
- Diag.emitError(OS, Mgr,
- "expected " + getDiagKindStr(Diag.Kind) + " \"" +
- Diag.Substring + "\" was not produced");
- OK = false;
- }
+bool SourceMgrDiagnosticVerifier::verify() {
+ // Check all buffers in SourceMgr registration order so diagnostics are
+ // reported deterministically and buffers that emitted no diagnostics (such as
+ // '.include'd files) are still scanned. Clear each buffer's vector rather
+ // than erasing map entries so a subsequent verify() call does not rescan
+ // buffers with Matched = false.
+ auto CheckExpectedDiags = [&](const ExpectedDiag &Diag) {
+ if (!Diag.Matched)
+ emitError(Diag, "expected " + getDiagKindStr(Diag.Kind) + " \"" +
+ Diag.Substring + "\" was not produced");
};
- for (auto &ExpectedDiagsPair : ExpectedDiagsPerFile)
- for (auto &Diag : ExpectedDiagsPair.second)
+ for (unsigned I = 0, E = Mgr.getNumBuffers(); I != E; ++I) {
+ const MemoryBuffer *Buf = Mgr.getMemoryBuffer(I + 1);
+ for (const ExpectedDiag &Diag : computeExpectedDiags(Buf))
CheckExpectedDiags(Diag);
- for (auto &Diag : ExpectedUnknownLocDiags)
+ ExpectedDiagsPerBuffer[Buf].clear();
+ }
+ for (const ExpectedDiag &Diag : ExpectedUnknownLocDiags)
CheckExpectedDiags(Diag);
- ExpectedDiagsPerFile.clear();
+ ExpectedUnknownLocDiags.clear();
return OK;
}
diff --git a/llvm/unittests/Support/CMakeLists.txt b/llvm/unittests/Support/CMakeLists.txt
index f9732ff4e0e4b..c5db78950d21b 100644
--- a/llvm/unittests/Support/CMakeLists.txt
+++ b/llvm/unittests/Support/CMakeLists.txt
@@ -90,6 +90,7 @@ add_llvm_unittest(SupportTests
SignalsTest.cpp
SipHashTest.cpp
SourceMgrTest.cpp
+ SourceMgrDiagnosticVerifierTest.cpp
SpecialCaseListTest.cpp
SuffixTreeTest.cpp
SwapByteOrderTest.cpp
diff --git a/llvm/unittests/Support/SourceMgrDiagnosticVerifierTest.cpp b/llvm/unittests/Support/SourceMgrDiagnosticVerifierTest.cpp
new file mode 100644
index 0000000000000..1e7f65e2c0b79
--- /dev/null
+++ b/llvm/unittests/Support/SourceMgrDiagnosticVerifierTest.cpp
@@ -0,0 +1,174 @@
+//===- unittests/Support/SourceMgrDiagnosticVerifierTest.cpp --------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Support/SourceMgrDiagnosticVerifier.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/SourceMgr.h"
+#include "llvm/Support/raw_ostream.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using MatchResult = SourceMgrDiagnosticVerifier::MatchResult;
+
+namespace {
+
+class SourceMgrDiagnosticVerifierTest : public testing::Test {
+protected:
+ SourceMgr SM;
+ std::string Output;
+ raw_string_ostream OS{Output};
+
+ const MemoryBuffer *addBuffer(StringRef Text,
+ StringRef BufferName = "test.s") {
+ unsigned ID = SM.AddNewSourceBuffer(
+ MemoryBuffer::getMemBuffer(Text, BufferName), SMLoc());
+ return SM.getMemoryBuffer(ID);
+ }
+};
+
+} // namespace
+
+TEST_F(SourceMgrDiagnosticVerifierTest, MatchAndVerifyIdempotent) {
+ const MemoryBuffer *Buf =
+ addBuffer("bad_op\n# expected-error at -1 {{invalid operand}}\n");
+ SourceMgrDiagnosticVerifier Verifier(SM, OS);
+
+ SMLoc Loc = SMLoc::getFromPointer(Buf->getBufferStart());
+ SMDiagnostic Diag =
+ SM.GetMessage(Loc, SourceMgr::DK_Error, "invalid operand");
+ EXPECT_EQ(Verifier.process(Diag), MatchResult::Matched);
+ EXPECT_TRUE(Verifier.verify());
+ EXPECT_EQ(Output, "");
+
+ // Calling verify() a second time (e.g. from a destructor after an explicit
+ // verify() call) must not re-parse the buffer with Matched = false.
+ EXPECT_TRUE(Verifier.verify());
+ EXPECT_EQ(Output, "");
+}
+
+TEST_F(SourceMgrDiagnosticVerifierTest, UnknownLocation) {
+ addBuffer("# expected-warning at unknown {{no location warning}}\n");
+ SourceMgrDiagnosticVerifier Verifier(SM, OS);
+
+ SMDiagnostic Diag =
+ SM.GetMessage(SMLoc(), SourceMgr::DK_Warning, "no location warning");
+ EXPECT_EQ(Verifier.process(Diag), MatchResult::Matched);
+ EXPECT_TRUE(Verifier.verify());
+ EXPECT_EQ(Output, "");
+}
+
+TEST_F(SourceMgrDiagnosticVerifierTest, UnmatchedUnknownLocation) {
+ addBuffer("# expected-error at unknown {{missing unknown error}}\n");
+ SourceMgrDiagnosticVerifier Verifier(SM, OS);
+
+ EXPECT_FALSE(Verifier.verify());
+ EXPECT_NE(
+ StringRef(Output).find(
+ "error: expected error \"missing unknown error\" was not produced"),
+ StringRef::npos);
+}
+
+TEST_F(SourceMgrDiagnosticVerifierTest, ReportUnexpectedFalse) {
+ const MemoryBuffer *Buf =
+ addBuffer("op\n# expected-error at -1 {{expected msg}}\n");
+ SourceMgrDiagnosticVerifier Verifier(SM, OS);
+
+ // An unexpected diagnostic with ReportUnexpected = false is ignored and does
+ // not fail verification.
+ SMLoc Loc = SMLoc::getFromPointer(Buf->getBufferStart());
+ SMDiagnostic UnexpectedDiag =
+ SM.GetMessage(Loc, SourceMgr::DK_Warning, "unrelated warning");
+ EXPECT_EQ(Verifier.process(UnexpectedDiag, /*ReportUnexpected=*/false),
+ MatchResult::Ignored);
+
+ SMDiagnostic ExpectedDiag =
+ SM.GetMessage(Loc, SourceMgr::DK_Error, "expected msg");
+ EXPECT_EQ(Verifier.process(ExpectedDiag, /*ReportUnexpected=*/false),
+ MatchResult::Matched);
+ EXPECT_TRUE(Verifier.verify());
+ EXPECT_EQ(Output, "");
+}
+
+TEST_F(SourceMgrDiagnosticVerifierTest, NullBufferWithHasLocIsUnexpected) {
+ addBuffer("# expected-error {{anything}}\n");
+ SourceMgrDiagnosticVerifier Verifier(SM, OS);
+
+ EXPECT_EQ(Verifier.process(SourceMgr::DK_Error, /*HasLoc=*/true,
+ /*Buf=*/nullptr, 1, "anything"),
+ MatchResult::Unexpected);
+ EXPECT_FALSE(Verifier.verify());
+}
+
+TEST_F(SourceMgrDiagnosticVerifierTest,
+ MultipleExpectedDiagsOnSameLinePreferUnmatched) {
+ const MemoryBuffer *Buf =
+ addBuffer("op\n"
+ "# expected-error at -1 {{invalid operand}}\n"
+ "# expected-error at -2 {{invalid operand}}\n");
+ SourceMgrDiagnosticVerifier Verifier(SM, OS);
+
+ SMLoc Loc = SMLoc::getFromPointer(Buf->getBufferStart());
+ SMDiagnostic Diag =
+ SM.GetMessage(Loc, SourceMgr::DK_Error, "invalid operand");
+ EXPECT_EQ(Verifier.process(Diag), MatchResult::Matched);
+ EXPECT_EQ(Verifier.process(Diag), MatchResult::Matched);
+ EXPECT_TRUE(Verifier.verify());
+ EXPECT_EQ(Output, "");
+}
+
+TEST_F(SourceMgrDiagnosticVerifierTest, LineZeroAndLeadingZeroOffsets) {
+ const MemoryBuffer *Buf =
+ addBuffer("// expected-error at +02 {{decimal leading zero}}\n"
+ "// expected-remark at -2 {{implicit module at line 0}}\n"
+ "op\n");
+ SourceMgrDiagnosticVerifier Verifier(SM, OS);
+
+ EXPECT_EQ(Verifier.process(SourceMgr::DK_Remark, /*HasLoc=*/true, Buf,
+ /*LineNo=*/0, "implicit module at line 0"),
+ MatchResult::Matched);
+ EXPECT_EQ(Verifier.process(SourceMgr::DK_Error, /*HasLoc=*/true, Buf,
+ /*LineNo=*/3, "decimal leading zero"),
+ MatchResult::Matched);
+ EXPECT_TRUE(Verifier.verify());
+ EXPECT_EQ(Output, "");
+}
+
+TEST_F(SourceMgrDiagnosticVerifierTest, OutOfBoundsLineOffsets) {
+ addBuffer("op\n"
+ "# expected-error at -3 {{before start}}\n"
+ "# expected-error at +5 {{past end}}\n");
+ SourceMgrDiagnosticVerifier Verifier(SM, OS);
+
+ EXPECT_FALSE(Verifier.verify());
+ EXPECT_NE(StringRef(Output).find(
+ "error: line offset '-3' is before the start of the file"),
+ StringRef::npos);
+ EXPECT_NE(StringRef(Output).find(
+ "error: line offset '+5' is past the end of the file"),
+ StringRef::npos);
+}
+
+TEST_F(SourceMgrDiagnosticVerifierTest, AlreadyMatchedDoesNotTriggerNearMiss) {
+ const MemoryBuffer *Buf = addBuffer("op\n# expected-error at -1 {{operand}}\n");
+ SourceMgrDiagnosticVerifier Verifier(SM, OS);
+
+ SMLoc Loc = SMLoc::getFromPointer(Buf->getBufferStart());
+ EXPECT_EQ(
+ Verifier.process(SM.GetMessage(Loc, SourceMgr::DK_Error, "bad operand")),
+ MatchResult::Matched);
+ // A subsequent unexpected note containing the same substring should be
+ // reported as Unexpected (not as a NearMiss against the already-satisfied
+ // expected-error).
+ EXPECT_EQ(
+ Verifier.process(SM.GetMessage(Loc, SourceMgr::DK_Note, "note operand")),
+ MatchResult::Unexpected);
+ EXPECT_FALSE(Verifier.verify());
+ EXPECT_NE(StringRef(Output).find("note: note operand"), StringRef::npos);
+ EXPECT_EQ(StringRef(Output).find("diagnostic emitted when expecting"),
+ StringRef::npos);
+}
diff --git a/llvm/utils/gn/secondary/llvm/unittests/Support/BUILD.gn b/llvm/utils/gn/secondary/llvm/unittests/Support/BUILD.gn
index 168b609279cab..7569f7e6fcbb9 100644
--- a/llvm/utils/gn/secondary/llvm/unittests/Support/BUILD.gn
+++ b/llvm/utils/gn/secondary/llvm/unittests/Support/BUILD.gn
@@ -93,6 +93,7 @@ unittest("SupportTests") {
"SignalsTest.cpp",
"SipHashTest.cpp",
"SourceMgrTest.cpp",
+ "SourceMgrDiagnosticVerifierTest.cpp",
"SpecialCaseListTest.cpp",
"SuffixTreeTest.cpp",
"SwapByteOrderTest.cpp",
diff --git a/mlir/lib/IR/Diagnostics.cpp b/mlir/lib/IR/Diagnostics.cpp
index ae678e1c44dfe..5f1a86e1ed495 100644
--- a/mlir/lib/IR/Diagnostics.cpp
+++ b/mlir/lib/IR/Diagnostics.cpp
@@ -640,8 +640,9 @@ namespace mlir {
namespace detail {
struct SourceMgrDiagnosticVerifierHandlerImpl {
SourceMgrDiagnosticVerifierHandlerImpl(
+ llvm::SourceMgr &srcMgr, raw_ostream &out,
SourceMgrDiagnosticVerifierHandler::Level level)
- : level(level) {}
+ : verifier(srcMgr, out), level(level) {}
/// The generic SourceMgr-based verifier core that this handler delegates
/// scanning and matching of 'expected-*' diagnostics to.
@@ -671,13 +672,7 @@ static StringRef getDiagKindStr(DiagnosticSeverity kind) {
SourceMgrDiagnosticVerifierHandler::SourceMgrDiagnosticVerifierHandler(
llvm::SourceMgr &srcMgr, MLIRContext *ctx, raw_ostream &out, Level level)
: SourceMgrDiagnosticHandler(srcMgr, ctx, out),
- impl(new SourceMgrDiagnosticVerifierHandlerImpl(level)) {
- // Compute the expected diagnostics for each of the current files in the
- // source manager.
- for (unsigned i = 0, e = mgr.getNumBuffers(); i != e; ++i)
- (void)impl->verifier.computeExpectedDiags(out, mgr,
- mgr.getMemoryBuffer(i + 1));
-
+ impl(new SourceMgrDiagnosticVerifierHandlerImpl(srcMgr, out, level)) {
// The base class registered a handler that prints every diagnostic. The
// verifier takes its place: it consumes diagnostics and reports unexpected
// ones itself.
@@ -697,7 +692,7 @@ SourceMgrDiagnosticVerifierHandler::~SourceMgrDiagnosticVerifierHandler() {
/// diagnostics were emitted. This return success if all diagnostics were
/// verified correctly, failure otherwise.
LogicalResult SourceMgrDiagnosticVerifierHandler::verify() {
- return success(impl->verifier.verify(os, mgr));
+ return success(impl->verifier.verify());
}
std::unique_ptr<ScopedDiagnosticHandler>
@@ -725,7 +720,7 @@ void SourceMgrDiagnosticVerifierHandler::process(LocationAttr loc,
using MatchResult = llvm::SourceMgrDiagnosticVerifier::MatchResult;
MatchResult result = impl->verifier.process(
- os, mgr, getDiagKind(kind), /*hasLoc=*/static_cast<bool>(fileLoc), buf,
+ getDiagKind(kind), /*hasLoc=*/static_cast<bool>(fileLoc), buf,
fileLoc ? fileLoc.getLine() : 0, msg, reportUnexpected);
if (result == MatchResult::Unexpected)
emitDiagnostic(loc, "unexpected " + getDiagKindStr(kind) + ": " + msg,
More information about the llvm-branch-commits
mailing list