[PATCH] D60386: FileCheck [6/12]: Introduce numeric variable definition

[Thomas Preud'homme via Phabricator via llvm-commits]

thopre marked 2 inline comments as done.
thopre added inline comments.


================
Comment at: llvm/trunk/docs/CommandGuide/FileCheck.rst:630-632
+Important note: In its current implementation, a numeric expression cannot use
+a numeric variable defined on the same line.
+
----------------
hliao wrote:
> hliao wrote:
> > thopre wrote:
> > > hliao wrote:
> > > > Just curious why we have such a restriction. Could someone elaborate more?
> > > > 
> > > > That makes numeric checks on the same line must be written in split like this
> > > > 
> > > > CHECK: [[#VAR:...]]
> > > > CHECK-SAME: [[#VAR + 1]]
> > > > 
> > > > instead of
> > > > 
> > > > CHECK: [[#VAR:...]] [[#VAR + 1]]
> > > > 
> > > > That's quite verbose.
> > > This is not currently done for 2 reasons:
> > > 
> > >   # The first reason is that the current code assumes the value of numeric expression is known at match time but in the case [[#VAR:]] [[#VAR+1]] the value VAR+1 is not known since VAR is not known yet. I have a patch planned to address that later in the patch series [1]
> > >   # The more serious problem is how to deal with input such as 10 12 13 and the CHECK [[#VAR:]] {{.*}} [[#VAR+1]] check. Here you'd want the CHECK to be successful and VAR to be assigned the value 12. However FileCheck with the aforementioned patch will ask the regex engine to match the input against the regex "([0-9]+) ([0-9]+)" and will then check the captured text to verify that the numeric expression is verified. However that regex will capture 10 and 12, being the first 2 numbers. Now given the .* how to look at all other possible matches? We would need help from the regex engine but the API only gives us a match/no match answer with captured text.
> > > 
> > > [1] https://reviews.llvm.org/D60392 which needs serious rebasing
> > > 
> > > Hope this answers your question.
> > 1. I wish [1] could be landed as soon as possible. So far, numeric substitution is inconsistent with the previous string substitution. The later allows the use and def in the same line. E.g. the following works
> >   ```
> >   11 11
> >   CHECK: [[VAR:[0-9]+]] [[VAR]]
> >   ```
> > 2. I haven't looked into the numeric support in detail. We should reference the original string block in FileCheck. A similar pattern works for the string block:
> >   ```
> >   11 22 22
> >   CHECK: [[VAR:[0-9]+]] [[VAR]]
> >   ```
> > Any details on why numeric block works differently from string block?
> > 
> Fix the formatting of the examples aforesaid:
> 
> ```
> 11 11
> CHECK: [[VAR:[0-9]+]] [[VAR]]
> ```
> 
> ```
> 11 22 22
> CHECK: [[VAR:[0-9]+]] [[VAR]]
> ```
> 
> both of them work for string blocks.
That's because it gets translated into "([0-9]+) \1" for the regex engine, i.e. string substitution can be expressed in terms of regex while numeric substitution cannot always. The current restriction is to only accept cases that can be expressed at the regex level.

The reason I've kept the patch to life part of the restriction last is that I'm not yet convinced it is a good thing to do. Take my example again but as a CHECK-NOT:

CHECK-NOT: [[#VAR:]] [[#VAR+1]]

This would be accepted after the last patch but if presented with the input "10 12 13" this would *not* give an error because the corresponding regex ("([0-9]+) ([0-9]+)") would match "10 12" which does not verify the +1 relation and thus the CHECK-NOT would be satisfied, despite the "12 13" bits. Given the potential for errors I'm leaning toward discarding the last patch at the moment.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60386/new/

https://reviews.llvm.org/D60386