[llvm-branch-commits] [llvm] [FileCheck][NFC] Complete FileCheckDiag class hierarchy (PR #195571)
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"
Date: Sun, 3 May 2026 18:01:23 -0400
Subject: [PATCH 1/4] [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?
[llvm-branch-commits] [llvm] [FileCheck][NFC] Complete FileCheckDiag class hierarchy (PR #195571)
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"
Date: Sun, 3 May 2026 18:01:23 -0400
Subject: [PATCH 1/4] [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?
[llvm-branch-commits] [llvm] [FileCheck][NFC] Complete FileCheckDiag class hierarchy (PR #195571)
@@ -125,111 +125,192 @@ 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,
MaskRay wrote:
Your call, but the `FCDK_` prefix seems verbose for an enum nested in
FileCheckDiag.
You can also hoist this to an `enum class`.
https://github.com/llvm/llvm-project/pull/195571
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [FileCheck][NFC] Complete FileCheckDiag class hierarchy (PR #195571)
https://github.com/MaskRay approved this pull request. https://github.com/llvm/llvm-project/pull/195571 ___ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [FileCheck][NFC] Complete FileCheckDiag class hierarchy (PR #195571)
@@ -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(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(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(Diag).getNote();
+break;
}
- llvm_unreachable_internal("unexpected match type");
+ if (Diag.isError()) {
MaskRay wrote:
The old code asserts
```
assert((!FiltersAsError || !Note.empty()) &&
"expected error diagnostic to have note");
```
Should this assert Res.FiltersAsError instead?
https://github.com/llvm/llvm-project/pull/195571
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [FileCheck][NFC] Complete FileCheckDiag class hierarchy (PR #195571)
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"
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?
[llvm-branch-commits] [llvm] [FileCheck][NFC] Complete FileCheckDiag class hierarchy (PR #195571)
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"
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?
[llvm-branch-commits] [llvm] [FileCheck][NFC] Complete FileCheckDiag class hierarchy (PR #195571)
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"
Date: Sun, 3 May 2026 18:01:23 -0400
Subject: [PATCH 1/2] [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?
[llvm-branch-commits] [llvm] [FileCheck][NFC] Complete FileCheckDiag class hierarchy (PR #195571)
github-actions[bot] wrote: :warning: C/C++ code formatter, clang-format found issues in your code. :warning: You can test this locally with the following command: ``bash git-clang-format --diff origin/main HEAD --extensions cpp,h -- llvm/include/llvm/FileCheck/FileCheck.h llvm/lib/FileCheck/FileCheck.cpp llvm/lib/FileCheck/FileCheckImpl.h llvm/unittests/FileCheck/FileCheckTest.cpp llvm/utils/FileCheck/FileCheck.cpp --diff_from_common_commit `` :warning: The reproduction instructions above might return results for more than one PR in a stack if you are using a stacked PR workflow. You can limit the results by changing `origin/main` to the base branch/commit you want to compare against. :warning: View the diff from clang-format here. ``diff diff --git a/llvm/utils/FileCheck/FileCheck.cpp b/llvm/utils/FileCheck/FileCheck.cpp index 459e13cd2..3088777fa 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(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] = `` https://github.com/llvm/llvm-project/pull/195571 ___ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [llvm] [FileCheck][NFC] Complete FileCheckDiag class hierarchy (PR #195571)
llvmorg-github-actions[bot] wrote:
@llvm/pr-subscribers-testing-tools
Author: Joel E. Denny (jdenny-ornl)
Changes
This patch depends on PR #195569 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.
---
Patch is 38.69 KiB, truncated to 20.00 KiB below, full version:
https://github.com/llvm/llvm-project/pull/195571.diff
5 Files Affected:
- (modified) llvm/include/llvm/FileCheck/FileCheck.h (+229-75)
- (modified) llvm/lib/FileCheck/FileCheck.cpp (+41-41)
- (modified) llvm/lib/FileCheck/FileCheckImpl.h (+1-3)
- (modified) llvm/unittests/FileCheck/FileCheckTest.cpp (+14-13)
- (modified) llvm/utils/FileCheck/FileCheck.cpp (+91-67)
``diff
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 supp
[llvm-branch-commits] [llvm] [FileCheck][NFC] Complete FileCheckDiag class hierarchy (PR #195571)
https://github.com/jdenny-ornl created https://github.com/llvm/llvm-project/pull/195571 This patch depends on PR #195569 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. >From 25f8dbdb87750286f2a003334de9ec7b723f00d8 Mon Sep 17 00:00:00 2001 From: "Joel E. Denny" Date: Sun, 3 May 2026 18:01:23 -0400 Subject: [PATCH] [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 FileChec
