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.
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.
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.
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
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?
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.
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.
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?
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.
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).
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?
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.