[PATCH] D47106: [FileCheck] Make CHECK-DAG non-overlapping

Paul Robinson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 3 08:58:05 PDT 2018


probinson added a comment.

Regarding the DAG-NOT-DAG reordering diagnostic:  Rereading the comments, and the llvm-dev discussion, are we agreed that we can eliminate it?  That is, the first DAG group defines some match range, the second DAG group defines a disjoint match range, and the NOT group looks at the text from the end of the first match range to the start of the second match range.  No attempt to diagnose ordering problems.

I also have some fussy comments about the torture test inline.



================
Comment at: test/FileCheck/check-dag-overlap-torture.txt:24
+; EndAfterEnd-NOT:  {{.}}
+; EndAfterEnd-SAME: ){{$}}
+
----------------
The latest revision of the conceptual model says we won't allow SAME or NEXT after DAG.  For purposes of determining valid/invalid directive sequences, the NOT wouldn't count.  I think you still get the intended result without the SAME suffix.
I see you're wanting to (in effect) have intra-line DAG, but DAG doesn't provide that guarantee.



================
Comment at: test/FileCheck/check-dag-overlap-torture.txt:36
+; EndAfterEnd-NOT:  {{.}}
+; EndAfterEnd-SAME: ){{$}}
+
----------------
This is the first set of checks where there's a difference in behavior depending on overlap or not.  Given the test is trying to exercise exactly that, the first two sets of checks aren't useful.  (They might be appropriate for a general DAG functionality test, but not here.)


================
Comment at: test/FileCheck/check-dag-overlap-torture.txt:42
+; EndAfterEnd-NOT:  {{.}}
+; EndAfterEnd-SAME: ){{$}}
+
----------------
This set of checks appears to be redundant with the previous `>xyz` set (the overlap is 3 characters instead of 1 but so what).


================
Comment at: test/FileCheck/check-dag-overlap-torture.txt:60
+; EndAfterEnd-NOT:  {{.}}
+; EndAfterEnd-SAME: ){{$}}
+
----------------
This set of checks appears to be redundant with the previous `c<mno>xyz` set.


================
Comment at: test/FileCheck/check-dag-overlap-torture.txt:67
+; RUN:               -check-prefix=EndRightAfterEnd
+; RUN: FileCheck -input-file %s %s -check-prefix=EndRightAfterEnd
+
----------------
I skimmed the rest of the checks, and offhand the comments on the EndAfterEnd part likely apply to the other parts:  (a) don't bother with sets of checks that don't overlap; (b) don't bother with redundant sets of checks that vary only in the number of overlapping characters.


https://reviews.llvm.org/D47106





More information about the llvm-commits mailing list