[FileCheck] Implement --ignore-case option.

[Kai]

Summary

The FileCheck utility is enhanced to support a --ignore-case
option. This is useful in cases where the output of Unix tools
differs in case (e.g. case not specified by Posix).

[alexander-shaposhnikov]

Oh, this is painful.
I'm curious - what do you think about some other options, e.g. about extending FileCheck to add support for case insensitiveness,
but maybe smb else might have a better idea

[jhenderson]

In D68146#1686536, @alexshap wrote:

Oh, this is painful.
I'm curious - what do you think about some other options, e.g. about extending FileCheck to add support for case insensitiveness,
but maybe smb else might have a better idea

I agree it's painful, and I think a FileCheck option is ultimately the best solution, but as an interim non-terrible solution, how about using tr to convert the od output from upper-case to lower-case:

# RUN: od -t x1 %t | tr '[:upper:]' '[:lower:]' | FileCheck ...

You can also use tr a-z A-Z in theory, but that relies on posix conformity of the od tool, which from experience I don't think we can rely on (e.g. I know that at least one version of od used on the Greendragon buildbots that doesn't allow command-lines to be ordered in somewhat arbitrary ways). I think therefore the character classes might be more reliable.

[MaskRay]

The command od -t x is used to dump data in hex format. The LIT tests assumes that the hex characters are in lowercase. However, there are also platforms which use uppercase letter. The test cases are changed to accept both formats.

Can you just name the platform? Is it AIX? Please give some examples of its output. IIRC it does not support -t x4.

Please also add -A n to the od command line. POSIX says "The format of the input offset is unspecified."

[rupprecht]

I think updating FileCheck to allow case-insensitive matchers is a good way to fix this. It may be tricky but it's the right thing to do.

[Kai]

Yeah, I agree that this is painful. I'll have a look at FileCheck because this really feels like the right solution.
I am not on AIX but z/OS USS.

[Kai]

FileCheck has now a new option --ignore-case.
I updated the test cases to use the new option. I touched only test cases which have the lower case/upper case letter problem. I also did not add -A n because some tests also check for the address.

[thopre]

In D68146#1691237, @Kai wrote:

FileCheck has now a new option --ignore-case.
I updated the test cases to use the new option. I touched only test cases which have the lower case/upper case letter problem. I also did not add -A n because some tests also check for the address.

Can you split the FileCheck change into a separate revision? Please add some testing (both unittest and regular test if possible).

[MaskRay]

Thanks for adding the new option --ignore-case. This will be very useful, though it is not clear from the option name that it uses a lowercase pattern:

size_t Pos = IgnoreCase ? Buffer.find_lower(FixedStr)
                        : Buffer.find(FixedStr);

To make it really --ignore-case, the pattern should also be changed to lowercase. It may be worth splitting the patch into two

1. [FileCheck] Add --ignore-case ... and add tests to test/FileCheck/
2. Fix od tests with the new FileCheck option.

llvm/lib/Support/FileCheckImpl.h

432

bool IgnoreCase = false; and delete the member initializer below.

[jhenderson]

I'm pleasantly surprised by how simple that FileCheck change is! You also need to update the FileCheck documentation (see llvm/docs/CommandGuide/FileCheck.rst) for the new option.

[jakehehrlich]

Seems like a good idea to me!

[Kai]

Added a test case and documentation for the --ignore-case option.
Removed the changed test cases.

[Kai]

In D68146#1691246, @MaskRay wrote:

To make it really --ignore-case, the pattern should also be changed to lowercase.

I am not sure which pattern I have missed. The fixed string match uses the lowercase find and the regex match uses the Regex::IgnoreCase flag.

[thopre]

llvm/test/FileCheck/check-ignore-case.txt

17

s/5/6/ ?

[Kai]

Change single digit in test case to form a sequence.

[thopre]

LGTM but I'll let others comment.

[arichardson]

LGTM

llvm/test/FileCheck/check-ignore-case.txt

23

newline?

[Kai]

Added newline at end of test case.

[jhenderson]

llvm/test/FileCheck/check-ignore-case.txt

23

Nit: no new line at EOF.

[Kai]

llvm/test/FileCheck/check-ignore-case.txt

23

When I download the raw file and look at it in hex mode, then the last byte is 0x0A. That's a new line at EOF, isn't it?

[rupprecht]

Mostly LGTM, just a couple test nits. Thanks for fixing FileCheck!

In D68146#1701152, @Kai wrote:

In D68146#1691246, @MaskRay wrote:

To make it really --ignore-case, the pattern should also be changed to lowercase.

I am not sure which pattern I have missed. The fixed string match uses the lowercase find and the regex match uses the Regex::IgnoreCase flag.

StringRef::find_lower() is a bad name for something that should really be called StringRef::find_case_insensitive(), which I think is the confusion here.

llvm/test/FileCheck/check-ignore-case.txt

2

Can you split up these lines with descriptions of what each one does? e.g.

## Check that ...
# RUN: FileCheck ...

## Check that other thing ...
# RUN: FileCheck ...

6

Can you add a check that case is ignored for -implicit-check-not? e.g. not FileCheck -ignore-case -implicit-check-not=sTrinG

[Kai]

Added a test for CHECK-SAME and for -implicit-check-not.

[MaskRay]

llvm/utils/FileCheck/FileCheck.cpp

53

Use case-insensitive matching? (To be consistent with the doc).

[Kai]

Changed description of option.

[MaskRay]

llvm/test/FileCheck/check-ignore-case.txt

2

independent of case. Or, how about case insensitively?

Please also add full stops in these comments.

6

-input-file=%s makes it clear -input-file accepts an argument.

I think @jhenderson may prefer --input-file=%s. Binary utilities (almost) exclusively use double-dashed forms now.

One bad thing with -input-file is that it is incompatible with POSIX Utility Syntax Guideline 5 (grouped options). Though FileCheck is not a POSIX utility and it does not necessarily abide by the rules...

18

sed /^#/d

44

Consider using a regex for 22.

[Kai]

Updated test case:

• changed description
• options now use double dash
• replaced sed expression
• added a regex to match column

[MaskRay]

LGTM, but please wait for another reviewer to take a final look.

[thakis]

The test fails on Linux: http://lab.llvm.org:8011/builders/clang-cmake-x86_64-sde-avx512-linux/builds/28537/steps/ninja%20check%201/logs/FAIL%3A%20LLVM%3A%3Acheck-ignore-case.txt If it takes a while to fix please revert while you investigate.

Also, https://github.com/llvm/llvm-project/commit/dfd2b6f07fc40a190335f580d8a965bbebfe94df looks like you touched ~all lines in docs/CommandGuide/FileCheck.rst and llvm/include/llvm/Support/FileCheck.h Maybe you converted them to windows line endings? If so, please undo that. (Maybe revert and reland with fixed line endings so that the diff for the actual change is readable.)

[gribozavr]

This change broke tests: http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/19066

I reverted it in r374359. Please run ninja check-all before committing.

[rupprecht]

In D68146#1703693, @thakis wrote:

The test fails on Linux: http://lab.llvm.org:8011/builders/clang-cmake-x86_64-sde-avx512-linux/builds/28537/steps/ninja%20check%201/logs/FAIL%3A%20LLVM%3A%3Acheck-ignore-case.txt If it takes a while to fix please revert while you investigate.

Also, https://github.com/llvm/llvm-project/commit/dfd2b6f07fc40a190335f580d8a965bbebfe94df looks like you touched ~all lines in docs/CommandGuide/FileCheck.rst and llvm/include/llvm/Support/FileCheck.h Maybe you converted them to windows line endings? If so, please undo that. (Maybe revert and reland with fixed line endings so that the diff for the actual change is readable.)

As described in http://llvm.org/docs/GettingStarted.html#checkout-llvm-from-git, the right way to checkout the repository on windows is:

% git clone --config core.autocrlf=false https://github.com/llvm/llvm-project.git

This is the second time I've reviewed a change like this without realizing, would appreciate tips if this could be more visible in Phab somehow...

[Kai]

In D68146#1704250, @rupprecht wrote:

In D68146#1703693, @thakis wrote:

The test fails on Linux: http://lab.llvm.org:8011/builders/clang-cmake-x86_64-sde-avx512-linux/builds/28537/steps/ninja%20check%201/logs/FAIL%3A%20LLVM%3A%3Acheck-ignore-case.txt If it takes a while to fix please revert while you investigate.

Also, https://github.com/llvm/llvm-project/commit/dfd2b6f07fc40a190335f580d8a965bbebfe94df looks like you touched ~all lines in docs/CommandGuide/FileCheck.rst and llvm/include/llvm/Support/FileCheck.h Maybe you converted them to windows line endings? If so, please undo that. (Maybe revert and reland with fixed line endings so that the diff for the actual change is readable.)

As described in http://llvm.org/docs/GettingStarted.html#checkout-llvm-from-git, the right way to checkout the repository on windows is:

% git clone --config core.autocrlf=false https://github.com/llvm/llvm-project.git

This is the second time I've reviewed a change like this without realizing, would appreciate tips if this could be more visible in Phab somehow...

Sorry, I really screwed this up. Not only that I managed to insert an additional character in the regex after my last test but the day before I had trouble with the repository (the COM1.o and COM2.o names on Windows) and cloned the repository again - obviously missing the CR/LF setting I had previously in.

[jhenderson]

llvm/test/FileCheck/check-ignore-case.txt

6

Thanks. I may have overly-strong feelings about the -/-- arguments, but I always find it off-putting seeing single-dash long options :)

FWIW, I'm less bothered when it's consistent within a test that only uses executables that don't need to care about single versus double dashes, such as this one.

23

I think this may have been a stale comment for the version I was viewing. At some point, Phabricator said "No new line at end of file" for me. Anyway, the final version you committed doesn't have the issue.

llvm/docs/CommandGuide/FileCheck.rst

 
 .. option:: --strict-whitespace
 
  By default, FileCheck canonicalizes input horizontal whitespace (spaces and
  tabs) which causes it to ignore these differences (a space will match a tab).
  The :option:`--strict-whitespace` argument disables this behavior. End-of-line
  sequences are canonicalized to UNIX-style ``\n`` in all modes.
 
+.. option:: --ignore-case
+
+  By default, FileCheck uses case-sensitive matching. This option causes
+  FileCheck to use case-insensitive matching.
+
 .. option:: --implicit-check-not check-pattern
 
   Adds implicit negative checks for the specified patterns between positive
   checks. The option allows writing stricter tests without stuffing them with
   ``CHECK-NOT``\ s.
 
   For example, "``--implicit-check-not warning:``" can be useful when testing
   diagnostic messages from tools that don't have an option similar to ``clang

llvm/include/llvm/Support/FileCheck.h

 /// Contains info about various FileCheck options.
 struct FileCheckRequest {
   std::vector<std::string> CheckPrefixes;
   bool NoCanonicalizeWhiteSpace = false;
   std::vector<std::string> ImplicitCheckNot;
   std::vector<std::string> GlobalDefines;
   bool AllowEmptyInput = false;
   bool MatchFullLines = false;
+  bool IgnoreCase = false;
   bool EnableVarScope = false;
   bool AllowDeprecatedDagOverlap = false;
   bool Verbose = false;
   bool VerboseVerbose = false;
 };
 
 //===----------------------------------------------------------------------===//
 // Summary of a FileCheck diagnostic.

llvm/lib/Support/FileCheck.cpp

FileCheckPattern::parseNumericSubstitutionBlock(
 
   return std::move(ExpressionAST);
 }
 
 bool FileCheckPattern::parsePattern(StringRef PatternStr, StringRef Prefix,
                                     SourceMgr &SM,
                                     const FileCheckRequest &Req) {
   bool MatchFullLinesHere = Req.MatchFullLines && CheckTy != Check::CheckNot;
+  IgnoreCase = Req.IgnoreCase;
 
   PatternLoc = SMLoc::getFromPointer(PatternStr.data());
 
   if (!(Req.NoCanonicalizeWhiteSpace && Req.MatchFullLines))
     // Ignore trailing whitespace.
     while (!PatternStr.empty() &&
            (PatternStr.back() == ' ' || PatternStr.back() == '\t'))
       PatternStr = PatternStr.substr(0, PatternStr.size() - 1);
Expected<size_t> FileCheckPattern::match(StringRef Buffer, size_t &MatchLen,
   if (CheckTy == Check::CheckEOF) {
     MatchLen = 0;
     return Buffer.size();
   }
 
   // If this is a fixed string pattern, just match it now.
   if (!FixedStr.empty()) {
     MatchLen = FixedStr.size();
-    size_t Pos = Buffer.find(FixedStr);
+    size_t Pos = IgnoreCase ? Buffer.find_lower(FixedStr)
+                            : Buffer.find(FixedStr);
     if (Pos == StringRef::npos)
       return make_error<FileCheckNotFoundError>();
     return Pos;
   }
 
   // Regex match.
 
   // If there are substitutions, we need to create a temporary string with the
for (const auto &Substitution : Substitutions) {
       InsertOffset += Value->size();
     }
 
     // Match the newly constructed regex.
     RegExToMatch = TmpStr;
   }
 
   SmallVector<StringRef, 4> MatchInfo;
-  if (!Regex(RegExToMatch, Regex::Newline).match(Buffer, &MatchInfo))
+  unsigned int Flags = Regex::Newline;
+  if (IgnoreCase)
+    Flags |= Regex::IgnoreCase;
+  if (!Regex(RegExToMatch, Flags).match(Buffer, &MatchInfo))
     return make_error<FileCheckNotFoundError>();
 
   // Successful regex match.
   assert(!MatchInfo.empty() && "Didn't get any match");
   StringRef FullMatch = MatchInfo[0];
 
   // If this defines any string variables, remember their values.
   for (const auto &VariableDef : VariableDefs) {

llvm/lib/Support/FileCheckImpl.h

class FileCheckPattern {
 
   Check::FileCheckType CheckTy;
 
   /// Line number for this CHECK pattern or None if it is an implicit pattern.
   /// Used to determine whether a variable definition is made on an earlier
   /// line to the one with this CHECK.
   Optional<size_t> LineNumber;
 
+  /// Ignore case while matching if set to true.
+  bool IgnoreCase = false;
+
 public:
   FileCheckPattern(Check::FileCheckType Ty, FileCheckPatternContext *Context,
                    Optional<size_t> Line = None)
       : Context(Context), CheckTy(Ty), LineNumber(Line) {}
 
   /// \returns the location in source code.
   SMLoc getLoc() const { return PatternLoc; }
 

llvm/test/FileCheck/check-ignore-case.txt

+# RUN: FileCheck -ignore-case -match-full-lines -check-prefix=FULL -input-file %s %s
+# RUN: FileCheck -ignore-case -check-prefix=REGEX -input-file %s %s
+# RUN: FileCheck -ignore-case -check-prefix=PAT -DPATTERN="THIS is the" -input-file %s %s
+# RUN: FileCheck -ignore-case -input-file %s %s
+
+this is the STRING to be matched
+
+# FULL: tHis iS The String TO be matched
+# REGEX: s{{TRing}}
+# PAT: [[PATTERN]] string
+
+Loop 1
+lOop 2
+loOp 3
+looP 4
+loop 5
+LOOP 5
+BREAK
+
+# CHECK-COUNT-6: LOop {{[0-9]}}
+# CHECK-NOT: loop
+# CHECK-NEXT: break
 No newline at end of file

llvm/utils/FileCheck/FileCheck.cpp

static cl::alias CheckPrefixesAlias(
     cl::NotHidden,
     cl::desc(
         "Alias for -check-prefix permitting multiple comma separated values"));
 
 static cl::opt<bool> NoCanonicalizeWhiteSpace(
     "strict-whitespace",
     cl::desc("Do not treat all horizontal whitespace as equivalent"));
 
+static cl::opt<bool> IgnoreCase(
+    "ignore-case",
+    cl::desc("Ignore case in comparisons"));
+
 static cl::list<std::string> ImplicitCheckNot(
     "implicit-check-not",
     cl::desc("Add an implicit negative check with this pattern to every\n"
              "positive check. This can be used to ensure that no instances of\n"
              "this pattern occur which are not matched by a positive pattern"),
     cl::value_desc("pattern"));
 
 static cl::list<std::string>
int main(int argc, char **argv) {
 
   Req.AllowEmptyInput = AllowEmptyInput;
   Req.EnableVarScope = EnableVarScope;
   Req.AllowDeprecatedDagOverlap = AllowDeprecatedDagOverlap;
   Req.Verbose = Verbose;
   Req.VerboseVerbose = VerboseVerbose;
   Req.NoCanonicalizeWhiteSpace = NoCanonicalizeWhiteSpace;
   Req.MatchFullLines = MatchFullLines;
+  Req.IgnoreCase = IgnoreCase;
 
   if (VerboseVerbose)
     Req.Verbose = true;
 
   FileCheck FC(Req);
   if (!FC.ValidateCheckPrefixes()) {
     errs() << "Supplied check-prefix is invalid! Prefixes must be unique and "
               "start with a letter and contain only alphanumeric characters, "