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

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


probinson added inline comments.


================
Comment at: test/FileCheck/check-dag-overlap-torture.txt:36
+; EndAfterEnd-NOT:  {{.}}
+; EndAfterEnd-SAME: ){{$}}
+
----------------
jdenny wrote:
> probinson wrote:
> > jdenny wrote:
> > > probinson wrote:
> > > > 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.)
> > > I intended for this test file to exhaustively exercise overlap detection.  That includes checking for both false positives and false negatives.
> > > 
> > > I see no reason to eliminate checking for false positives just because those cases behaved the same when we permitted overlaps.  I want to check that overlap detection works correctly in all cases now.
> > Do you anticipate deleting this test when the overlap feature is removed?  If so, I won't argue about its content (much).
> No, I primarily mean for this test to exercise overlap detection for DAG, which is important as long as non-overlapping is enforced.
That's fine (clearly we'd remove the RUN lines with the deprecated switch, when that gets removed).
But in that case, there are already a bunch of tests for non-overlapping DAG lines in other check-dag*.txt tests, and it distracts from the focus of this test to repeat them here. That would be the first two sets of checks in EndAfterEnd, and corresponding sets in the other cases.
(Or if you think the existing DAG tests in the other files are inadequate, beef up those tests, and keep this one focused on overlap.) 


https://reviews.llvm.org/D47106





More information about the llvm-commits mailing list