RFC: FileCheck Enhancements

7. Wildcard for prefixes - If some statements should be checked
regardless prefix, it should be used //{{*}}, //{{*}}-NEXT,
//{{*}}-SAME and etc.
8. Prefix with regular expressions - If statement should be
checked if prefix matches some regular expression, it should be used
{{regex}}:, {{regex}}-NEXT and etc.

More information in file
https://docs.google.com/document/d/1wAKNzU7-S2EeK1-aADwgP8dEiKfByKNazonybCQW3zs/edit?usp=sharing.

I noticed the google doc stated that multi-line patterns are not
supported. That's not actually the case, although it's a bit obscure:
the [[:space:]] character-class will match EOL and allow you to write
a multi-line CHECK.

Also: CHECK-SAME.

Whoops... I misread... Ignore me.

Jon

I support this. I found it very confusing when
; CHECK-NEXT: br
to check for a branch instruction also matched a line eg.
%2 = add i32 %subregion, i32 5

However, it is a breaking change and therefore might require changes
in existing tests.

Michael

From: llvm-dev [mailto:llvm-dev-bounces@lists.llvm.org] On Behalf Of
Michael Kruse via llvm-dev
Sent: Friday, June 03, 2016 11:03 AM
To: James Y Knight
Cc: llvm-dev
Subject: Re: [llvm-dev] RFC: FileCheck Enhancements

2016-05-26 18:13 GMT+02:00 James Y Knight via llvm-dev
<llvm-dev@lists.llvm.org>:
> For "6. Check statement for words only" -- I think it might be better to
> just make that be the ONLY behavior, rather than an additional option --
if
> you intended to end a match in the middle of a word, stick {{[^ ]*}} on
it.

I support this. I found it very confusing when
; CHECK-NEXT: br
to check for a branch instruction also matched a line eg.
%2 = add i32 %subregion, i32 5

However, it is a breaking change and therefore might require changes
in existing tests.

Definitely. I've seen things like "CHECK: lea" which is intentionally
insensitive to "leal" versus "leaq" as the generated X86 instruction.
It would take a while to find all such tests and make them tolerate a
word-based FileCheck.
--paulr

Hi all,

Now all discussed enhancements are divided into separate patches. Moreover I have found mistake which reduces performance. Now it's fixed.

List of patches (for those interested):
Repeats in regular expressions - https://reviews.llvm.org/D22342
Including files - https://reviews.llvm.org/D22344
Expressions repeat for CHECK and CHECK-NEXT - https://reviews.llvm.org/D22345
CHECK-LABEL-DAG - https://reviews.llvm.org/D22348
CHECK-WORD - https://reviews.llvm.org/D22353
prefixes-regular expressions - https://reviews.llvm.org/D22401
pattern templates - https://reviews.llvm.org/D22403.

Thanks,
Elena.

Please add llvm-commits as a subscriber to all of these reviews
so everyone has a chance to see/comment on them.
Thanks,
--paulr

Hi Elena,

Sorry if my question is something you’ve previously addressed. I didn’t comment on the reviews themselves as I don’t have any expertise with the implementation of FileCheck. However, much like most LLVM developers, I actively use the tool.

A brief look at the patches does not show updates to the documentation. I’m sure that many LLVM developers like myself will gladly use new features if they prove useful, but we just need to know about them. Rather than the source, I personally get information about how to use it from here: http://llvm.org/docs/CommandGuide/FileCheck.html.

So I would say that for most of us, if the feature is not documented there, it is as if it doesn’t exist.

Is there a separate patch (or a plan to put one up for review) for the documentation updates?

Thanks,

Nemanja

Adding llvm-commits after the fact ends up in the mailing list not archiving the context of the reviews.
If these patches are fresh (no significant review occurred), they should be closed and new revisions need to be opened, with llvm-commits as a subscriber from the start.

Please add llvm-commits as a subscriber to all of these reviews
so everyone has a chance to see/comment on them.

Adding llvm-commits after the fact ends up in the mailing list not archiving the context of the reviews.
If these patches are fresh (no significant review occurred), they should be closed and new revisions need to be opened, with llvm-commits as a subscriber from the start.

Ah, oops. I've started reviews of a few of the patches. If you plan on re-creating the reviews, please mark the relevant in-line comments on these as "Done" so it's easier to track what's changed: D22353, D22403, D22344.

thanks
vedant

Hi,

I thought documentation will be changed after review if some changes are accepted. But as I need to create new patches because of next comments, I’ll add changes in documentation.

Thanks,

Elena.

It does send a patch to the list if you upload a new diff to the existing review after adding the list as a subscriber. That seems the best way to go usually?

Thank you, James.

Then I’ll update all patches.

It still doesn’t include the original context/description of the patch, though. Yes, you can copy/paste that in, though.

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

It seems certainly possible, and has been asked before as well, and my memory of the answer is along the line of “patch welcome”.
(I.e. some community member are spending time to customize and maintain our fabricator instance, but they have some real work to do as well, so we’re resource limited).

Cool, thanks!

PS. I did some searching for this, and it seems there's no "easy" way to do this customisation via the project config. Probably something that has to be done on the server side. (See ◳ Q268 There is a way to "personlize" arcanist default template?).

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?

Thanks, Elena.

And I also want to ask again about possible change of regexp library. There are a lot of comments that some changes in FileCheck are useless, because they may be replaced by using some features of regular expressions, but they are not supported by current library.

I don't know a lot about modern C++ regexp library, but there are:
1. PCRE(pcre.h)
2. std::regex. It has no necessary features. So it can't be taken.
3. There is regex library in boost.
4. Also there is regex library in poco.

Are there any other variants?

Thanks,
Elena.

I had actually just meant in my comments to update the library we have, not switch to a brand new one.

But as I understood current library is quite old and it is branched from OpenBSD library, and there are no new versions.

Do you suggest to make changes in this version?