[PATCH] D68061: [docs] Document pattern of using CHECK-SAME to skip irrelevant lines
James Henderson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Sep 27 01:22:39 PDT 2019
jhenderson added inline comments.
================
Comment at: llvm/docs/CommandGuide/FileCheck.rst:324
+would still pass because the "``CHECK: Value: 1``" line would match the value
+from ``baz``. To fix this, you could add ``CHECK-NEXT`` matchers for every
+``FieldN:`` line, but that would be verbose, and need to be updated when
----------------
thopre wrote:
> rupprecht wrote:
> > greened wrote:
> > > This is what CHECK-LABEL is for. This isn't a very motivating example. Is there a better one?
> > I don't think CHECK-LABEL works in this case. CHECK-LABEL requires unique lines, and the only thing unique here would be "Name: <...>" which is not the thing we have a problem checking.
> >
> > e.g. something like:
> > ```
> > CHECK: Name: foo
> > CHECK-LABEL: Value: 1
> > ```
> > would fail on the example text above because `foo` and `baz` both have a value of one. Similarly,
> > ```
> > CHECK-LABEL: Name: foo
> > CHECK: Value: 1
> > ```
> > has the same false-passing problem as described in these added docs.
> >
> > Of course, you could add tests for every name/value pair (and using check-label on the names to make error messages clearer), but then the test overall becomes not a unit test: you really just want to write a unit test that verifies foo has a value of 1; you don't want to assert the universe to do that.
> >
> > I don't use check-label very often, I'm mostly relying on docs for my understanding. Could you clarify your comment if I'm missing something? Also: if I'm not missing something, let me know how I can make the wording here more clear.
> CHECK-LABEL will constraints the CHECK directive between them to match between the two posititions where the CHECK-LABEL match. In other word, if the name in your example are uniques then
>
> CHECK-LABEL: Name: foo
> CHECK: Value: 1
> CHECK-LABEL: Name: bar
>
> will guarantee that the Value: 1 will match something in the block of text between "Name: foo" and "Name: bar" or fail to match. Now if you only care about foo, your solution avoids adding a CHECK-LABEL for bar which might not be easy if you do not know the relative ordering of the blocks for foo and bar.
>
> So I do think your use case is useful, but perhaps you should remove the parts about bar and baz (my preference) or mention that the order of blocks cannot be relied upon (which might be solved in some future time by having something like CHECK-LABEL-DAG/CHECK-REGION-DAG which would force to update this text to my first suggestion).
Perhaps replacing the reference to `baz` with something like "a later symbol in the output" would be sufficient? It might also be worth noting why CHECK-LABEL isn't appropriate in this case (e.g. unknown ordering).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68061/new/
https://reviews.llvm.org/D68061
More information about the llvm-commits
mailing list