[FileCheck] Added --match-full-lines-leading and --match-full-lines-trailing option

[jhenderson]

What's the motivation for this change?

[strimo378]

For clang AST dump test cases, we often have code like

TranslationUnitDecl
`-FunctionDecl <line:2:1, line:5:1> line:2:5 func 'int (int, int)'
  |-ParmVarDecl <col:10, col:14> col:14 used x 'int'
  |-ParmVarDecl <col:17, col:21> col:21 used y 'int'
  `-CompoundStmt <col:24, line:5:1>
    `-ReturnStmt <line:3:5, col:16>
      `-BinaryOperator <col:12, col:16> 'int' '+'
        |-ImplicitCastExpr <col:12> 'int' <LValueToRValue>
        | `-DeclRefExpr <col:12> 'int' lvalue ParmVar 0xbb44db0 'x' 'int'
        `-ImplicitCastExpr <col:16> 'int' <LValueToRValue>
          `-DeclRefExpr <col:16> 'int' lvalue ParmVar 0xbb44e30 'y' 'int'

with matching rules like

FunctionDecl {{.*}} func 'int (int, int)'

with --match-full-lines we would need to add the ugly trailing stuff to test cases.

[jhenderson]

This comment was removed by jhenderson.

[jhenderson]

I'm not really seeing from your example why you need --match-full-lines at all. Are you just looking for the {{^}} and {{$}} regex patterns?

[strimo378]

--match-full-lines is require to check that nothing trailing is missing. Current approach is to add in 100 lines {{$}}

[MaskRay]

I also don't see a need to add any new option.

https://github.com/llvm/llvm-project/blob/638865c8f93989c5f7a237417356c920d55e0136/llvm/test/tools/llvm-readobj/ELF/file-headers.test#L6

We often check things this way:
# RUN: llvm-readobj --file-headers %t.i386 | FileCheck %s --strict-whitespace --match-full-lines -DFILE=%t.i386 --check-prefix I386

#      I386:File: [[FILE]]
# I386-NEXT:Format: elf32-i386
# I386-NEXT:Arch: i386
# I386-NEXT:AddressSize: 32bit
# I386-NEXT:LoadName: <Not found>
# I386-NEXT:ElfHeader {
# I386-NEXT:  Ident {
# I386-NEXT:    Magic: (7F 45 4C 46)

Note that : is aligned, different from the style we use when --strict-whitespace --match-full-lines are unused.

[jhenderson]

In D158164#4595420, @strimo378 wrote:

--match-full-lines is require to check that nothing trailing is missing. Current approach is to add in 100 lines {{$}}

Okay, I think I see your point. You are simply looking to find a way to apply that regex pattern to every line, much like --match-full-lines adds an implicit {{^}} and {{$}} around each line. Is that correct? And you don't want to use --match-full-lines because of the noisy prefixes (hence I'm not sure @MaskRay's suggestion will work).

I'm not strongly opposed to the new options. However, they should be a) documented, and b) I think the names are wrong, since they contradict themselves - trailing only means not "full-lines". I would suggest that the names might be --match-leading-lines and --match-trailing-lines or something to that effect.

@jdenny/@thopre, any thoughts?

[jdenny]

In D158164#4598113, @jhenderson wrote:

In D158164#4595420, @strimo378 wrote:

--match-full-lines is require to check that nothing trailing is missing. Current approach is to add in 100 lines {{$}}

Okay, I think I see your point. You are simply looking to find a way to apply that regex pattern to every line, much like --match-full-lines adds an implicit {{^}} and {{$}} around each line. Is that correct? And you don't want to use --match-full-lines because of the noisy prefixes (hence I'm not sure @MaskRay's suggestion will work).

I'm not strongly opposed to the new options.

On one hand, I want to argue this patch will contribute to the difficulty of writing careful FileCheck directives because, when looking at a FileCheck pattern, there would be even more possible strictness levels that might be in effect. I already get confused when moving among tests: sometimes I think I still have the strictness of -match-full-lines or -strict-whitespace as I saw in the last test, but in the test I'm currently editing I don't, so I accidentally write a weak pattern.

On the other hand, this patch might encourage people to make tests more strict. They might not if strictness requires extending hundreds of lines with {{^}}, {{$}}, or {{.*}} (useful when -match-full-lines is already in effect but the start/end of a line is not interesting).

At least the implementation is simple.

However, they should be a) documented,

And tested in FileCheck's own test suite.

and b) I think the names are wrong, since they contradict themselves - trailing only means not "full-lines". I would suggest that the names might be --match-leading-lines and --match-trailing-lines or something to that effect.

Agreed except those names confuse me too. :-) It sounds like matching multiple leading/trailing lines in a file.

Maybe -strict-line-start and -strict-line-end?

[MaskRay]

llvm/utils/FileCheck/FileCheck.cpp

94

For opt-in options, we typically omit cl::init(false)

[strimo378]

Alright, thank you all for your detailed feedback. I'll address all comments and provide an updated patch with documentation, test case and the proposed changes. Unfortunately, I am going on vacating right now and I am not sure if I am in the mood of development. So it could take some weeks ...

llvm/include/llvm/FileCheck/FileCheck.h

 struct FileCheckRequest {
   std::vector<StringRef> CheckPrefixes;
   std::vector<StringRef> CommentPrefixes;
   bool NoCanonicalizeWhiteSpace = false;
   std::vector<StringRef> ImplicitCheckNot;
   std::vector<StringRef> GlobalDefines;
   bool AllowEmptyInput = false;
   bool AllowUnusedPrefixes = false;
-  bool MatchFullLines = false;
+  bool MatchFullLinesTrailing = false;
+  bool MatchFullLinesLeading = false;
   bool IgnoreCase = false;
   bool IsDefaultCheckPrefix = false;
   bool EnableVarScope = false;
   bool AllowDeprecatedDagOverlap = false;
   bool Verbose = false;
   bool VerboseVerbose = false;
 };
 

llvm/lib/FileCheck/FileCheck.cpp

if (DefEnd != StringRef::npos) {
     DefinedNumericVariable = *ParseResult;
   }
 
   return std::move(ExpressionPointer);
 }
 
 bool Pattern::parsePattern(StringRef PatternStr, StringRef Prefix,
                            SourceMgr &SM, const FileCheckRequest &Req) {
-  bool MatchFullLinesHere = Req.MatchFullLines && CheckTy != Check::CheckNot;
+  bool MatchFullLinesLeadingHere =
+      Req.MatchFullLinesLeading && CheckTy != Check::CheckNot;
+  bool MatchFullLinesTrailingHere =
+      Req.MatchFullLinesTrailing && CheckTy != Check::CheckNot;
   IgnoreCase = Req.IgnoreCase;
 
   PatternLoc = SMLoc::getFromPointer(PatternStr.data());
 
-  if (!(Req.NoCanonicalizeWhiteSpace && Req.MatchFullLines))
+  if (!(Req.NoCanonicalizeWhiteSpace && Req.MatchFullLinesLeading))
     // Ignore trailing whitespace.
     while (!PatternStr.empty() &&
            (PatternStr.back() == ' ' || PatternStr.back() == '\t'))
       PatternStr = PatternStr.substr(0, PatternStr.size() - 1);
 
   // Check that there is something on the line.
   if (PatternStr.empty() && CheckTy != Check::CheckEmpty) {
     SM.PrintMessage(PatternLoc, SourceMgr::DK_Error,
bool Pattern::parsePattern(StringRef PatternStr, StringRef Prefix,
 
   // If literal check, set fixed string.
   if (CheckTy.isLiteralMatch()) {
     FixedStr = PatternStr;
     return false;
   }
 
   // Check to see if this is a fixed string, or if it has regex pieces.
-  if (!MatchFullLinesHere &&
+  if (!(MatchFullLinesLeadingHere || MatchFullLinesTrailingHere) &&
       (PatternStr.size() < 2 ||
        (!PatternStr.contains("{{") && !PatternStr.contains("[[")))) {
     FixedStr = PatternStr;
     return false;
   }
 
-  if (MatchFullLinesHere) {
+  if (MatchFullLinesLeadingHere) {
     RegExStr += '^';
     if (!Req.NoCanonicalizeWhiteSpace)
       RegExStr += " *";
   }
 
   // Paren value #0 is for the fully matched string.  Any new parenthesized
   // values add from there.
   unsigned CurParen = 1;
while (!PatternStr.empty()) {
     // Handle fixed string matches.
     // Find the end, which is the start of the next regex.
     size_t FixedMatchEnd =
         std::min(PatternStr.find("{{", 1), PatternStr.find("[[", 1));
     RegExStr += Regex::escape(PatternStr.substr(0, FixedMatchEnd));
     PatternStr = PatternStr.substr(FixedMatchEnd);
   }
 
-  if (MatchFullLinesHere) {
+  if (MatchFullLinesTrailingHere) {
     if (!Req.NoCanonicalizeWhiteSpace)
       RegExStr += " *";
     RegExStr += '$';
   }
 
   return false;
 }
 
if (CheckTy == Check::CheckBadCount) {
       SM.PrintMessage(SMLoc::getFromPointer(Buffer.data()), SourceMgr::DK_Error,
                       "invalid count in -COUNT specification on prefix '" +
                           UsedPrefix + "'");
       return true;
     }
 
     // Okay, we found the prefix, yay. Remember the rest of the line, but ignore
     // leading whitespace.
-    if (!(Req.NoCanonicalizeWhiteSpace && Req.MatchFullLines))
+    if (!(Req.NoCanonicalizeWhiteSpace && Req.MatchFullLinesTrailing))
       Buffer = Buffer.substr(Buffer.find_first_not_of(" \t"));
 
     // Scan ahead to the end of line.
     size_t EOL = Buffer.find_first_of("\n\r");
 
     // Remember the location of the start of the pattern, for diagnostics.
     SMLoc PatternLoc = SMLoc::getFromPointer(Buffer.data());
 

llvm/utils/FileCheck/FileCheck.cpp

static cl::opt<bool> AllowUnusedPrefixes(
     cl::desc("Allow prefixes to be specified but not appear in the test."));
 
 static cl::opt<bool> MatchFullLines(
     "match-full-lines", cl::init(false),
     cl::desc("Require all positive matches to cover an entire input line.\n"
              "Allows leading and trailing whitespace if --strict-whitespace\n"
              "is not also passed."));
 
+static cl::opt<bool> MatchFullLinesLeading(
+    "match-full-lines-leading", cl::init(false),
+    cl::desc("Require all positive matches to cover the entire trailing input\n"
+             "line. Allows trailing whitespace if --strict-whitespace is not\n"
+             "also passed."));
+
+static cl::opt<bool> MatchFullLinesTrailing(
+    "match-full-lines-trailing", cl::init(false),
+    cl::desc("Require all positive matches to cover the entire trailing input\n"
+             "line. Allows leading whitespace if --strict-whitespace is not\n"
+             "also passed."));
+
 static cl::opt<bool> EnableVarScope(
     "enable-var-scope", cl::init(false),
     cl::desc("Enables scope for regex variables. Variables with names that\n"
              "do not start with '$' will be reset at the beginning of\n"
              "each CHECK-LABEL block."));
 
 static cl::opt<bool> AllowDeprecatedDagOverlap(
     "allow-deprecated-dag-overlap", cl::init(false),
int main(int argc, char **argv) {
 
   Req.AllowEmptyInput = AllowEmptyInput;
   Req.AllowUnusedPrefixes = AllowUnusedPrefixes;
   Req.EnableVarScope = EnableVarScope;
   Req.AllowDeprecatedDagOverlap = AllowDeprecatedDagOverlap;
   Req.Verbose = Verbose;
   Req.VerboseVerbose = VerboseVerbose;
   Req.NoCanonicalizeWhiteSpace = NoCanonicalizeWhiteSpace;
-  Req.MatchFullLines = MatchFullLines;
+  Req.MatchFullLinesLeading = MatchFullLines || MatchFullLinesLeading;
+  Req.MatchFullLinesTrailing = MatchFullLines || MatchFullLinesTrailing;
   Req.IgnoreCase = IgnoreCase;
 
   if (VerboseVerbose)
     Req.Verbose = true;
 
   FileCheck FC(Req);
   if (!FC.ValidateCheckPrefixes())
     return 2;