[llvm-dev] RFC: FileCheck Enhancements

Thu Sep 1 15:30:33 PDT 2016

Hi,

I also like proposed grammar. One minor correction, I guess you want to
allow pattern use with parameters as well:

 PATTERN_USE <- '[[' '@' IDENT ARGLIST? ']]' ;

On Wed, Aug 31, 2016 at 10:56 PM, Vedant Kumar via llvm-dev <
llvm-dev at lists.llvm.org> wrote:

> > At first I thought that `register(n)` was some sort of macro, but if it
> is suppose to be equivalent to the example above of what we do “today”,
> then using “register(“1”)” is supposed to “capture” the ‘r’ part of the
> register on the first match.
>
>
> This is a problem with my suggested gramamr (specifically: it doesn't
> provide a
> way to use defined patterns to capture text for later reference).  Fred
> brought
> up that he was confused by this too.
>
> One way to fix it is to use '@' before using patterns. I'll recap the
> suggested
> grammar and work through another example. Here's how you'd define a
> pattern:
>
>   CHECK-DEFINE-PATTERN: car(make, model, year): {{Found a }} make model
> {{, from }} year
>
> And here's how you'd use it, *without* capturing the text into a variable:
>
>   CHECK: [[@car("Honda", "Accord", "2009")]]
>
> (Note, this matches the text: "Found a Honda Accord, from 2009".)
>
> To use a pattern and capture the matched text in "MY_CAR", you'd write:
>
>   CHECK: [[MY_CAR @car("Honda", "Accord", "2009")]]
>
> This gives us an unambiguous way to capture text matched via a defined
> pattern,
> which is visually distinct from how normal regex-based capturing works.
>
> Note that subsequent uses of "MY_CAR" should work as expected (i.e, you
> can do
> '[[MY_CAR]]' later in the test).
>
> Revised grammar:
>
>    ACTION <- CHECK ':' MATCH '\n' ;
>    ACTION <- CHECK-DEFINE-PATTERN ':' IDENT PARAMLIST? ':'
> PATTERN_ELEMENT* '\n' ;
>    PARAMLIST <- '(' IDENT (',' IDENT)* ')' ;
>    PATTERN_ELEMENT <- IDENT | REGEX;
>    MATCH <- (TEXT | REGEX | PATTERN_USE | VAR)* ;
>    REGEX <- '{{' POSIX_REGEX '}}' ;
>    PATTERN_USE <- '[[' '@' IDENT ']]' ;
>    VAR <- '[[' IDENT ':' POSIX_REGEX ']]' ;
>    VAR <- '[[' IDENT '@' IDENT ARGLIST? ']]' ;
>    ARGLIST <- '(' ARG (',' ARG)* ')' ;
>    ARG <- "([^"]|\\")*" ;
>
> vedant
>
> > On Aug 31, 2016, at 9:11 AM, Mehdi Amini <mehdi.amini at apple.com> wrote:
> >
> >>
> >> On Aug 24, 2016, at 4:46 PM, Vedant Kumar via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
> >>
> >>>
> >>> On Aug 24, 2016, at 2:04 AM, Elena Lepilkina <
> Elena.Lepilkina at synopsys.com> wrote:
> >>>
> >>> Hi all,
> >>>
> >>> Some discussions and comments were made in reviews. Much time has
> already passed since last comment and uploading changed patches. I made
> small summary report about features here, because there are some doubts
> about syntax of some features and changes in patches and it'll be great to
> know more opinions.
> >>>
> >>> 1. FileCheck Enhancement - CHECK-WORD (https://reviews.llvm.org/D22353
> )
> >>> I replace special directives by flag --check-word, which turns on mode
> for each directive in file.  It's obvious that this mode can be replaced
> using \b assert, but current regexp library doesn't have support of this
> assert and I have no answer to question about possibility of change current
> library.
> >>> There was the discussion about that such mode can be made default, but
> there were doubts about necessity of a lot of work for changing existing
> tests.
> >>> And I made experiment which proves that a lot of old tests will be
> failed with such mode on.
> >>> Expected Passes    : 15810
> >>> Expected Failures  : 125
> >>> Unsupported Tests  : 195
> >>> Unexpected Passes  : 4
> >>> Unexpected Failures: 1128
> >>
> >> I would rather not introduce churn in our tests by turning on
> --check-word by
> >> default. I'm also not convinced that turning on --check-word at the
> test level
> >> is the right move: having a CHECK-WORD directive is more flexible, and
> not a
> >> serious inconvenience (as compared to writing "CHECK").
> >>
> >>
> >>>
> >>> 2. FileCheck Enhancement - pattern templates (
> https://reviews.llvm.org/D22403)
> >>> There are some doubts about syntax of templates. I agree that use of
> \#, \:, \= is quite different from usual characters in FileCheck and was
> chosen because of same approach for escaping in regexp. Adrian Prantl
> suggested to use double-brackets "[[" to escape.
> >>> Old syntax:
> >>> \#(template_name) - use of template 'template_name'. It can occur in
> CHECK-PATTERN line, when description of one template includes other
> templates described before. (Without quote, I don't know how escape # here)
> >>> \:(Variable_name)- template variable with name 'variable_name'
> >>> \:(variable_name)\=(value) - current value of template variable(it's
> needed when you use template with variables).
> >>> Suggested new syntax:
> >>> [[#template_name]] - use of template 'template_name'. It can occur in
> CHECK-PATTERN line, when description of one template includes other
> templates described before. (Without quote, I don't know how escape # here)
> >>> [[:Variable_name]] - template variable with name 'variable_name'
> >>> [[:variable_name=value]] - current value of template variable(it's
> needed when you use template with variables).
> >>> It'll be great to hear more opinions and suggestions about syntax. May
> be someone has really good ideas. Then I'll be able to change it.
> >>
> >> First, I want to recap the FileCheck workflow Elena is proposing:
> >>
> >>  1. Define patterns using the CHECK-DEFINE-PATTERN directive. Defined
> patterns
> >>     have a name and may optionally have parameters.
> >>
> >>  2. Use defined patterns in the usual CHECK* directives.
> >>
> >> This is similar to how FileCheck patterns work already. The difference
> is
> >> that the patterns are defined using a dedicated directive, *not* when
> the
> >> pattern is first encountered. E.g, here is what you can do today:
> >>
> >>  // RUN: echo "%r1 %r2" | FileCheck %s
> >>  // CHECK: %[[register:[a-z]+]]1
> >>  // CHECK-SAME: %[[register]]2
> >>
> >> With the proposed changes, we'll be able to write something like:
> >>
> >>  // RUN: echo "%cmp %cmp" | FileCheck %s
> >>  // CHECK-DEFINE-PATERN: register(n): {{[a-z]+}}n
> >>  // CHECK: %[[register("1")]]
> >>  // CHECK-SAME: %[[register("2")]]
> >
> > At first I thought that `register(n)` was some sort of macro, but if it
> is suppose to be equivalent to the example above of what we do “today”,
> then using “register(“1”)” is supposed to “capture” the ‘r’ part of the
> register on the first match.
> > So you cannot reuse “register()” later to capture another expression.
> For instance:
> >
> >  // RUN: FileCheck %s
> >  // CHECK-DEFINE-PATERN: register(n): {{[a-z]+}}n
> >  // CHECK: %[[register("1")]]
> >  // CHECK-SAME: %[[register("2")]]
> >  // CHECK: %[[register("1")]]
> >  // CHECK-SAME: %[[register("2")]]
> > %r1 %r2
> > %reg1 %reg2 #will fail here.
> >
> >
> > If true, I find this confusing, if not, I missed something in your
> example.
> >
> > —
> > Mehdi
> >
> >
> >
> >>
> >> I saw "something like" because we haven't decided on the syntax for
> defining
> >> and using patterns (that's what this thread is for). Briefly, here's
> the syntax
> >> I'd like to use:
> >>
> >>  // Defining patterns.
> >>  CHECK-DEFINE-PATERN: <Name>(<Ident>, ...)?: <Pattern>
> >>
> >>  Where <Pattern> is a list of <PatternElement>, and a <PatternElement>
> is
> >>  either a regex ('{{' POSIX_REGEX '}}') or an argument identifier
> (IDENT).
> >>
> >>  // Using patterns.
> >>  CHECK: [[<Name>(<Argument>, ...)?]]
> >>
> >> Fleshing this out some more, here is my candidate grammar (see the end
> of this
> >> email for the current grammar):
> >>
> >>  ACTION <- CHECK ':' MATCH '\n' ;
> >>  ACTION <- CHECK-DEFINE-PATTERN ':' IDENT PARAMLIST? ':'
> PATTERN_ELEMENT* '\n' ;
> >>  PARAMLIST <- '(' IDENT (',' IDENT)* ')' ;
> >>  PATTERN_ELEMENT <- IDENT ;
> >>  PATTERN_ELEMENT <- REGEX ;
> >>  MATCH <- ;
> >>  MATCH <- TEXT MATCH ;
> >>  MATCH <- REGEX MATCH ;
> >>  MATCH <- VAR MATCH ;
> >>  REGEX <- '{{' POSIX_REGEX '}}' ;
> >>  VAR <- '[[' IDENT ':' POSIX_REGEX ']]' ;
> >>  VAR <- '[[' IDENT ARGLIST? ']]' ;
> >>  ARGLIST <- '(' ARG (',' ARG)* ')' ;
> >>  ARG <- "([^"]|\\")*" ;
> >>
> >>
> >>> 3. FileCheck Enhancement - repeats in regular expressions (
> https://reviews.llvm.org/D22454), FileCheck Enhancement - Including files
> (https://reviews.llvm.org/D22500), FileCheck Enhancement - Expressions
> repeat for CHECK and CHECK-NEXT(https://reviews.llvm.org/D22501),
> FileCheck Enhancement - CHECK-LABEL-DAG(https://reviews.llvm.org/D22502),
> FileCheck Enhancement - prefixes-regular expressions (
> https://reviews.llvm.org/D22503)
> >>> There were no comments about these enhancements at all. Your opinions
> are very important.
> >>
> >> I personally am waiting for some version of D22403 to land in-tree
> before
> >> starting on the other reviews. This would help me gauge what others in
> the
> >> community are thinking and what they need.
> >>
> >>>
> >>> I hope that some of these changes will be useful for FileCheck users,
> so I need your opinions to get opportunity for review to be resumed.
> >>
> >> thanks,
> >> vedant
> >>
> >> Original FileCheck grammar (shamelessly copied from the grammar Adrian
> posted
> >> to D22403):
> >>
> >>  ACTION <- CHECK ':' MATCH '\n' ;
> >>  MATCH <- ;
> >>  MATCH <- TEXT MATCH ;
> >>  MATCH <- REGEX MATCH ;
> >>  MATCH <- VAR MATCH ;
> >>  REGEX <- '{{' POSIX_REGEX '}}' ;
> >>  VAR <- '[[' IDENT ':' POSIX_REGEX ']]' ;
> >>  VAR <- '[[' IDENT ']]' ;
> >>
> >>
> >>>
> >>> -----Original Message-----
> >>> From: llvm-dev [mailto:llvm-dev-bounces at lists.llvm.org] On Behalf Of
> Elena Lepilkina via llvm-dev
> >>> Sent: Wednesday, July 20, 2016 4:52 PM
> >>> To: vsk at apple.com
> >>> Cc: llvm-dev at lists.llvm.org
> >>> Subject: Re: [llvm-dev] RFC: FileCheck Enhancements
> >>>
> >>> List of last patches:
> >>>
> >>> 1. FileCheck Enhancement - CHECK-WORD (llvm-commits was added later as
> diff update) - https://reviews.llvm.org/D22353 2. FileCheck Enhancement -
> pattern templates(llvm-commits was added later as diff update) -
> https://reviews.llvm.org/D22403 3. FileCheck Enhancement - repeats in
> regular expressions (new review with llvm-commits) -
> https://reviews.llvm.org/D22454 4. FileCheck Enhancement - Including
> files (new review with llvm-commits) - https://reviews.llvm.org/D22500
> >>> 5. FileCheck Enhancement - Expressions repeat for CHECK and CHECK-NEXT
> (new review with llvm-commits)   - https://reviews.llvm.org/D22501
> >>> 6. FileCheck Enhancement - CHECK-LABEL-DAG (new review with
> llvm-commits)  - https://reviews.llvm.org/D22502 7. FileCheck Enhancement
> - prefixes-regular expressions (new review with llvm-commits) -
> https://reviews.llvm.org/D22503
> >>>
> >>> Thanks,
> >>> Elena.
> >>>
> >>> -----Original Message-----
> >>> From: vsk at apple.com [mailto:vsk at apple.com]
> >>> Sent: Tuesday, July 19, 2016 8:42 PM
> >>> To: Elena Lepilkina <Elena.Lepilkina at synopsys.com>
> >>> Cc: Dean Michael Berris <dean.berris at gmail.com>; Mehdi Amini <
> mehdi.amini at apple.com>; llvm-dev at lists.llvm.org
> >>> Subject: Re: [llvm-dev] RFC: FileCheck Enhancements
> >>>
> >>> Hi Elena,
> >>>
> >>>
> >>>> On Jul 19, 2016, at 6:36 AM, Elena Lepilkina via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
> >>>>
> >>>> Hi all,
> >>>>
> >>>> I made new patches for most of changes with llvm-commits subscriber.
> But two patches were updated, because there are a lot of comments (patch
> for CHECK-WORD and patch for templates pattern). Will it be ok?
> >>>
> >>> IMO it's fine to keep some of the original reviews if you don't want
> to discard/recreate their state.
> >>>
> >>> Please list the most up-to-date set of Phab URL's here, with a little
> note next to the ones which did not initially CC llvm-commits.
> >>>
> >>> Thanks again for working on this!
> >>>
> >>> vedant
> >>>
> >>>>
> >>>> Thanks, Elena.
> >>>>
> >>>> -----Original Message-----
> >>>> From: llvm-dev [mailto:llvm-dev-bounces at lists.llvm.org] On Behalf Of
> >>>> Dean Michael Berris via llvm-dev
> >>>> Sent: Tuesday, July 19, 2016 6:53 AM
> >>>> To: Mehdi Amini <mehdi.amini at apple.com>
> >>>> Cc: via llvm-dev <llvm-dev at lists.llvm.org>
> >>>> Subject: Re: [llvm-dev] RFC: FileCheck Enhancements
> >>>>
> >>>>
> >>>>> On 19 Jul 2016, at 04:18, Mehdi Amini via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
> >>>>>
> >>>>> We had a long thread about that a few weeks (months?) ago: the
> conclusion (as I remember) was roughly a guideline to “always start a new
> revision to have a proper mailing-list thread starting with context (i.e.
> patch description)”
> >>>>> (and my dissident minority opinion that it is only worth it if there
> >>>>> hasn’t been significant round of reviews going on on the existing
> >>>>> revision)
> >>>>>
> >>>>
> >>>> Pardon me for missing that discussion, this may have already been
> asked before: but is it possible to make arcanist default subscribe the
> correct commits mailing list in the process? This should make it at least
> harder to forget.
> >>>>
> >>>> Cheers
> >>>> _______________________________________________
> >>>> LLVM Developers mailing list
> >>>> llvm-dev at lists.llvm.org
> >>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
> >>>> _______________________________________________
> >>>> LLVM Developers mailing list
> >>>> llvm-dev at lists.llvm.org
> >>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
> >>>
> >>> _______________________________________________
> >>> LLVM Developers mailing list
> >>> llvm-dev at lists.llvm.org
> >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
> >>
> >> _______________________________________________
> >> LLVM Developers mailing list
> >> llvm-dev at lists.llvm.org
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160902/f8705666/attachment.html>