[llvm] ce68545 - [FileCheck] Fix --dump-input annotation sort per input line
Joel E. Denny via llvm-commits
llvm-commits at lists.llvm.org
Thu Apr 16 12:40:52 PDT 2020
Author: Joel E. Denny
Date: 2020-04-16T15:39:35-04:00
New Revision: ce685455e4500f9f4a6686b1667a132d2c8a3c12
URL: https://github.com/llvm/llvm-project/commit/ce685455e4500f9f4a6686b1667a132d2c8a3c12
DIFF: https://github.com/llvm/llvm-project/commit/ce685455e4500f9f4a6686b1667a132d2c8a3c12.diff
LOG: [FileCheck] Fix --dump-input annotation sort per input line
Without this patch, `--dump-input` annotations on a single input line
are sorted by the associated directive's check-file line. That seemed
fine because that's often identical to the order in which FileCheck
looks for matches for those directives.
The first problem is that an `--implicit-check-not` pattern has no
check-file line. The logical equivalent is sorting in command-line
order, but that's not implemented.
The second problem is that, unlike a directive, an
`--implicit-check-not` pattern applies at many points, between many
different pairs of directives. However, sorting in command-line order
gathers all its associated diagnostics together at one point in an
input line's list of annotations.
In general, it seems to be easier to understand FileCheck's logic when
annotations on a single input line are sorted in the order FileCheck
produced the associated diagnostics, so this patch makes that change.
As documented in the patch, the annotation sort order is also
especially relevant to `CHECK-LABEL`, `CHECK-NOT`, and `CHECK-DAG`, so
this patch updates or extends tests to check the sort makes sense for
them. (However, the sort for `CHECK-DAG` annotations should not
actually be altered by this patch.)
Reviewed By: thopre
Differential Revision: https://reviews.llvm.org/D77607
Added:
Modified:
llvm/test/FileCheck/dump-input-annotations.txt
llvm/utils/FileCheck/FileCheck.cpp
Removed:
################################################################################
diff --git a/llvm/test/FileCheck/dump-input-annotations.txt b/llvm/test/FileCheck/dump-input-annotations.txt
index a0fce27b42b0..f4f0d3ca6022 100644
--- a/llvm/test/FileCheck/dump-input-annotations.txt
+++ b/llvm/test/FileCheck/dump-input-annotations.txt
@@ -280,8 +280,8 @@
; EMP-NEXT: 3: world
; EMP-NEXT: empty:3 X~~~~ error: no match found
; EMP-NEXT: 4: label
-; EMP-NEXT: empty:3 ~~~~~
; EMP-V-NEXT: label:4 ^~~~~
+; EMP-NEXT: empty:3 ~~~~~
; EMP-NEXT: >>>>>>
; EMP-NOT: {{.}}
@@ -387,8 +387,8 @@
; NOT2-VV-NEXT: not:1 ~~~~~
; NOT2-NEXT: not:2 !~~~~ error: no match expected
; NOT2-NEXT: 3: again
-; NOT2-VV-NEXT: not:1 ~~
; NOT2-V-NEXT: check:3 ^~~
+; NOT2-VV-NEXT: not:1 ~~
; NOT2-NEXT: >>>>>>
; NOT2-NOT: {{.}}
@@ -446,6 +446,53 @@
; DAG-NEXT: >>>>>>
; DAG-NOT: {{.}}
+; Check sorting of annotations when the order of diagnostics across an input
+; line is
diff erent than the order of the associated directives in the check
+; file. Try cases when diagnostics' input ranges overlap but are not
+; identical to check how that affects sorting.
+
+; RUN: echo 'abc def abc def' > %t.in
+
+; RUN: echo 'CHECK-DAG: def' > %t.chk
+; RUN: echo 'CHECK-DAG: bc' >> %t.chk
+; RUN: echo 'CHECK-DAG: abc' >> %t.chk
+; RUN: echo 'CHECK-DAG: de' >> %t.chk
+; RUN: echo 'CHECK-DAG: def' >> %t.chk
+
+; RUN: %ProtectFileCheckOutput \
+; RUN: not FileCheck -dump-input=always -input-file %t.in %t.chk 2>&1 \
+; RUN: | FileCheck -match-full-lines %s -check-prefixes=DAG1L,DAG1L-Q \
+; RUN: -implicit-check-not='remark:'
+; RUN: %ProtectFileCheckOutput \
+; RUN: not FileCheck -dump-input=always -input-file %t.in %t.chk -v 2>&1 \
+; RUN: | FileCheck -match-full-lines %s -check-prefixes=DAG1L,DAG1L-V,DAG1L-VQ \
+; RUN: -implicit-check-not='remark:'
+; RUN: %ProtectFileCheckOutput \
+; RUN: not FileCheck -dump-input=always -input-file %t.in %t.chk -vv 2>&1 \
+; RUN: | FileCheck -match-full-lines %s -check-prefixes=DAG1L,DAG1L-V,DAG1L-VV \
+; RUN: -implicit-check-not='remark:'
+
+; Verbose diagnostics are suppressed but not errors.
+; DAG1L:{{.*}}error:{{.*}}
+
+; DAG1L:<<<<<<
+; DAG1L-NEXT: 1: abc def abc def
+; DAG1L-V-NEXT:dag:1 ^~~
+; DAG1L-V-NEXT:dag:2 ^~
+; DAG1L-VV-NEXT:dag:3'0 !~~ discard: overlaps earlier match
+; DAG1L-VQ-NEXT:dag:3 ^~~
+; DAG1L-VV-NEXT:dag:3'1 ^~~
+; DAG1L-VV-NEXT:dag:4'0 !~ discard: overlaps earlier match
+; DAG1L-VQ-NEXT:dag:4 ^~
+; DAG1L-VV-NEXT:dag:4'1 ^~
+; DAG1L-VV-NEXT:dag:5'0 !~~ discard: overlaps earlier match
+; DAG1L-VV-NEXT:dag:5'1 !~~ discard: overlaps earlier match
+; DAG1L-Q-NEXT:dag:5 X error: no match found
+; DAG1L-VQ-NEXT:dag:5 X error: no match found
+; DAG1L-VV-NEXT:dag:5'2 X error: no match found
+; DAG1L-NEXT:>>>>>>
+; DAG1L-NOT:{{.}}
+
;--------------------------------------------------
; CHECK-LABEL
;
@@ -536,22 +583,19 @@
; Verbose diagnostics are suppressed but not errors.
; IMPNOT:{{.*}}error:{{.*}}
-; FIXME: All occurrences of imp1, imp2, and imp3 are sorting after the first
-; directive. They should instead be sorted by when they execute.
-
; IMPNOT:<<<<<<
; IMPNOT-NEXT: 1: hello world again!
; IMPNOT-V-NEXT:check:1 ^~~
; IMPNOT-VV-NEXT:not:imp1 X
; IMPNOT-VV-NEXT:not:imp2 X
; IMPNOT-VV-NEXT:not:imp3 X
+; IMPNOT-V-NEXT:check:2 ^~~
; IMPNOT-VV-NEXT:not:imp1 X~~
; IMPNOT-VV-NEXT:not:imp2 X~~
; IMPNOT-VV-NEXT:not:imp3 X~~
+; IMPNOT-V-NEXT:check:3 ^
; IMPNOT-VV-NEXT:not:imp1 X~~~~~~~
; IMPNOT-VV-NEXT:not:imp2 X~~~~~~~
; IMPNOT-NEXT:not:imp3 !~~~~ error: no match expected
-; IMPNOT-V-NEXT:check:2 ^~~
-; IMPNOT-V-NEXT:check:3 ^
; IMPNOT-NEXT:>>>>>>
; IMPNOT-NOT:{{.}}
diff --git a/llvm/utils/FileCheck/FileCheck.cpp b/llvm/utils/FileCheck/FileCheck.cpp
index 6cfd0fd75878..ef6c62b5ab5b 100644
--- a/llvm/utils/FileCheck/FileCheck.cpp
+++ b/llvm/utils/FileCheck/FileCheck.cpp
@@ -241,11 +241,8 @@ static void DumpInputAnnotationHelp(raw_ostream &OS) {
/// An annotation for a single input line.
struct InputAnnotation {
- /// The check file line (one-origin indexing) where the directive that
- /// produced this annotation is located.
- unsigned CheckLine;
- /// The index of the match result for this check.
- unsigned CheckDiagIndex;
+ /// The index of the match result across all checks
+ unsigned DiagIndex;
/// The label for this annotation.
std::string Label;
/// What input line (one-origin indexing) this annotation marks. This might
@@ -253,7 +250,7 @@ struct InputAnnotation {
/// a non-initial fragment of a diagnostic that has been broken across
/// multiple lines.
unsigned InputLine;
- /// The column range (one-origin indexing, open end) in which to to mark the
+ /// The column range (one-origin indexing, open end) in which to mark the
/// input line. If InputEndCol is UINT_MAX, treat it as the last column
/// before the newline.
unsigned InputStartCol, InputEndCol;
@@ -300,6 +297,8 @@ BuildInputAnnotations(const SourceMgr &SM, unsigned CheckFileBufferID,
const std::vector<FileCheckDiag> &Diags,
std::vector<InputAnnotation> &Annotations,
unsigned &LabelWidth) {
+ // How many diagnostics have we seen so far?
+ unsigned DiagCount = 0;
// How many diagnostics has the current check seen so far?
unsigned CheckDiagCount = 0;
// What's the widest label?
@@ -307,12 +306,12 @@ BuildInputAnnotations(const SourceMgr &SM, unsigned CheckFileBufferID,
for (auto DiagItr = Diags.begin(), DiagEnd = Diags.end(); DiagItr != DiagEnd;
++DiagItr) {
InputAnnotation A;
+ A.DiagIndex = DiagCount++;
// Build label, which uniquely identifies this check result.
unsigned CheckBufferID = SM.FindBufferContainingLoc(DiagItr->CheckLoc);
auto CheckLineAndCol =
SM.getLineAndColumn(DiagItr->CheckLoc, CheckBufferID);
- A.CheckLine = CheckLineAndCol.first;
llvm::raw_string_ostream Label(A.Label);
Label << GetCheckTypeAbbreviation(DiagItr->CheckTy) << ":";
if (CheckBufferID == CheckFileBufferID)
@@ -323,19 +322,17 @@ BuildInputAnnotations(const SourceMgr &SM, unsigned CheckFileBufferID,
else
llvm_unreachable("expected diagnostic's check location to be either in "
"the check file or for an implicit pattern");
- A.CheckDiagIndex = UINT_MAX;
+ unsigned CheckDiagIndex = UINT_MAX;
auto DiagNext = std::next(DiagItr);
if (DiagNext != DiagEnd && DiagItr->CheckTy == DiagNext->CheckTy &&
DiagItr->CheckLoc == DiagNext->CheckLoc)
- A.CheckDiagIndex = CheckDiagCount++;
+ CheckDiagIndex = CheckDiagCount++;
else if (CheckDiagCount) {
- A.CheckDiagIndex = CheckDiagCount;
+ CheckDiagIndex = CheckDiagCount;
CheckDiagCount = 0;
}
- if (A.CheckDiagIndex != UINT_MAX)
- Label << "'" << A.CheckDiagIndex;
- else
- A.CheckDiagIndex = 0;
+ if (CheckDiagIndex != UINT_MAX)
+ Label << "'" << CheckDiagIndex;
Label.flush();
LabelWidth = std::max((std::string::size_type)LabelWidth, A.Label.size());
@@ -366,8 +363,7 @@ BuildInputAnnotations(const SourceMgr &SM, unsigned CheckFileBufferID,
if (DiagItr->InputEndCol == 1 && L == E)
break;
InputAnnotation B;
- B.CheckLine = A.CheckLine;
- B.CheckDiagIndex = A.CheckDiagIndex;
+ B.DiagIndex = A.DiagIndex;
B.Label = A.Label;
B.InputLine = L;
B.Marker = A.Marker;
@@ -392,35 +388,53 @@ static void DumpAnnotatedInput(raw_ostream &OS, const FileCheckRequest &Req,
OS << "Full input was:\n<<<<<<\n";
// Sort annotations.
- //
- // First, sort in the order of input lines to make it easier to find relevant
- // annotations while iterating input lines in the implementation below.
- // FileCheck diagnostics are not always reported and recorded in the order of
- // input lines due to, for example, CHECK-DAG and CHECK-NOT.
- //
- // Second, for annotations for the same input line, sort in the order of the
- // FileCheck directive's line in the check file (where there's at most one
- // directive per line) and then by the index of the match result for that
- // directive. The rationale of this choice is that, for any input line, this
- // sort establishes a total order of annotations that, with respect to match
- // results, is consistent across multiple lines, thus making match results
- // easier to track from one line to the next when they span multiple lines.
std::sort(Annotations.begin(), Annotations.end(),
[](const InputAnnotation &A, const InputAnnotation &B) {
+ // 1. Sort annotations in the order of the input lines.
+ //
+ // This makes it easier to find relevant annotations while
+ // iterating input lines in the implementation below. FileCheck
+ // does not always produce diagnostics in the order of input
+ // lines due to, for example, CHECK-DAG and CHECK-NOT.
if (A.InputLine != B.InputLine)
return A.InputLine < B.InputLine;
- if (A.CheckLine != B.CheckLine)
- return A.CheckLine < B.CheckLine;
- // FIXME: Sometimes CHECK-LABEL reports its match twice with
- // other diagnostics in between, and then diag index incrementing
- // fails to work properly, and then this assert fails. We should
- // suppress one of those diagnostics or do a better job of
- // computing this index. For now, we just produce a redundant
- // CHECK-LABEL annotation.
- // assert(A.CheckDiagIndex != B.CheckDiagIndex &&
- // "expected diagnostic indices to be unique within a "
- // " check line");
- return A.CheckDiagIndex < B.CheckDiagIndex;
+ // 2. Sort annotations in the temporal order FileCheck produced
+ // their associated diagnostics.
+ //
+ // This sort offers several benefits:
+ //
+ // A. On a single input line, the order of annotations reflects
+ // the FileCheck logic for processing directives/patterns.
+ // This can be helpful in understanding cases in which the
+ // order of the associated directives/patterns in the check
+ // file or on the command line either (i) does not match the
+ // temporal order in which FileCheck looks for matches for the
+ // directives/patterns (due to, for example, CHECK-LABEL,
+ // CHECK-NOT, or `--implicit-check-not`) or (ii) does match
+ // that order but does not match the order of those
+ // diagnostics along an input line (due to, for example,
+ // CHECK-DAG).
+ //
+ // On the other hand, because our presentation format presents
+ // input lines in order, there's no clear way to offer the
+ // same benefit across input lines. For consistency, it might
+ // then seem worthwhile to have annotations on a single line
+ // also sorted in input order (that is, by input column).
+ // However, in practice, this appears to be more confusing
+ // than helpful. Perhaps it's intuitive to expect annotations
+ // to be listed in the temporal order in which they were
+ // produced except in cases the presentation format obviously
+ // and inherently cannot support it (that is, across input
+ // lines).
+ //
+ // B. When diagnostics' annotations are split among multiple
+ // input lines, the user must track them from one input line
+ // to the next. One property of the sort chosen here is that
+ // it facilitates the user in this regard by ensuring the
+ // following: when comparing any two input lines, a
+ // diagnostic's annotations are sorted in the same position
+ // relative to all other diagnostics' annotations.
+ return A.DiagIndex < B.DiagIndex;
});
// Compute the width of the label column.
More information about the llvm-commits
mailing list