[FileCheck] Added [[@CLEAR:regex]] as a way to clear variables.

[tra]

Summary

The patch introduces special variable [[@CLEAR:regex]] which
always matches without consuming any input and undefines all
variables that match supplied regex.

This is useful for keeping individual tests in the same file hermetic.
Currently, it's very easy for a failing test to pass because it
happened to use a variable set by a preceding similar test.

[arsenm]

I always thought that a CHECK-LABEL should clear all defined variables

[arsenm]

In D30725#695081, @arsenm wrote:

I always thought that a CHECK-LABEL should clear all defined variables

It definitely does not now, I think it would be less burdensome than having to list every used variable (which is just another place you can make a typo) to clear it

[tra]

In D30725#695081, @arsenm wrote:

I always thought that a CHECK-LABEL should clear all defined variables

As far as I can tell it does not. E.g. this passes:

// RUN: FileCheck -check-prefix Y -input-file %s %s

op y2
op y2l
op y2 y3
; Y:  [[REGY:y[0-9]]]
; Y-LABEL:  op y2l
; Y:  op [[REGY]] y3

[jlebar]

It definitely does not now, I think it would be less burdensome than having to list every used variable (which is just another place you can make a typo) to clear it

Yeah, but cleaning that up globally in all our tests sounds like a recipe for disaster. And I'm sure we'll also find some tests where we really *do* want to keep regexps alive across CHECK-LABELS.

I suppose we could have a filecheck mode that behaves as you describe, but that also sounds worse.

Also, if you don't want to list the used variables, you can match on [[@CLEAR:.*]].

[arsenm]

In D30725#695102, @jlebar wrote:

It definitely does not now, I think it would be less burdensome than having to list every used variable (which is just another place you can make a typo) to clear it

Yeah, but cleaning that up globally in all our tests sounds like a recipe for disaster. And I'm sure we'll also find some tests where we really *do* want to keep regexps alive across CHECK-LABELS.

I suppose we could have a filecheck mode that behaves as you describe, but that also sounds worse.

Also, if you don't want to list the used variables, you can match on [[@CLEAR:.*]].

I think those are abuses of CHECK-LABEL then. The use case is to treat a region begun by a label as a separate error reporting region. The common case is for multiple independent testcase functions in a single file. The weird case that is checking matches between functions is a special case that I don't think should be using -LABEL

[jlebar]

I think those are abuses of CHECK-LABEL then.

I'm not sure I agree, but even if I do, I'm not volunteering to clean them up... That seems like a lot of pain for no gain.

[tra]

@arsenm

I think check-label and regex variables are orthogonal. CHECK-LABEL does separate chunks of input for matching and error reporting purposes, but regex variables are more akin to a preprocessor -- we use variable value to construct regex to match against the input. I don't think CHECK-LABEL is a good fit for hard boundary on variable lifetime.

Here's one (reasonable, I believe) use case for across-the-CHECK-LABEL vars:

; CHECK-LABEL: generated_function_foo
; CHECK: store something [[SHARED_OBJECT:generated_object_name_x[0-9]+]]
...
; CHECK-LABEL: generated_function_bar
; CHECK: x = load [[SHARED_OBJECT]]

[arsenm]

In D30725#695129, @tra wrote:

@arsenm

I think check-label and regex variables are orthogonal. CHECK-LABEL does separate chunks of input for matching and error reporting purposes, but regex variables are more akin to a preprocessor -- we use variable value to construct regex to match against the input. I don't think CHECK-LABEL is a good fit for hard boundary on variable lifetime.

Here's one (reasonable, I believe) use case for across-the-CHECK-LABEL vars:

; CHECK-LABEL: generated_function_foo
; CHECK: store something [[SHARED_OBJECT:generated_object_name_x[0-9]+]]
...
; CHECK-LABEL: generated_function_bar
; CHECK: x = load [[SHARED_OBJECT]]

If not CHECK-LABEL, then I think a CLEAR-ALL-VARIABLES would be more helpful/less burdensome than clearing individual vars (or maybe a CHECK-REGION combination of LABEL and clearing)

[tra]

@arsenm OK. CHECK-REGION (= CHECK-LABEL + clear all vars) would work for me.

Any other opinions?

[jlebar]

I like CHECK-REGION.

[hfinkel]

In D30725#695171, @tra wrote:

@arsenm OK. CHECK-REGION (= CHECK-LABEL + clear all vars) would work for me.

Any other opinions?

What if we divided the regex names into two classes: local and global. CHECK-LABEL cleared the local ones and the global ones stayed around? We could do something like have:

[[@NAME:[pattern]]]

bind a global name (and the existing ones without the @ are local).

[tra]

@hfinkel I like the global/local split. @ would clash with @LINE[-+]N expression we currently support, but we could use some other prefix instead. $ ?

[hfinkel]

In D30725#695187, @tra wrote:

@hfinkel I like the global/local split. @ would clash with @LINE[-+]N expression we currently support, but we could use some other prefix instead. $ ?

$ is certainly fine with me.

[tra]

Superseded by D30749 which implements Hal's idea.

docs/CommandGuide/FileCheck.rst

 
 .. code-block:: llvm
 
     ; CHECK: op [[REG:r[0-9]+]], [[REG]]
 
 Can be useful if you want the operands of ``op`` to be the same register,
 and don't care exactly which register it is.
 
+In order to clear previously defined variables you can match a special
+variable ``@CLEAR``. This variable always matches and does not consume
+any input. Instead, all variables that match supplied pattern get
+undefined. This is useful for keeping individual tests in the same
+file hermetic.
+
+.. code-block:: llvm
+
+   ; CHECK: test5:
+   ; CHECK:    notw [[REGISTER:%[a-z]+]]
+   ; CHECK:    andw {{.*}}[[REGISTER]]
+   ; CHECK: new_test: [[@CLEAR:R.*]]
+   ; -- Following line will always fail because REGISTER is no longer defined.
+   ; CHECK:    foo      {{.*}}[[REGISTER]]
+
 FileCheck Expressions
 ~~~~~~~~~~~~~~~~~~~~~
 
 Sometimes there's a need to verify output which refers line numbers of the
 match file, e.g. when testing compiler diagnostics.  This introduces a certain
 fragility of the match file structure, as "``CHECK:``" lines contain absolute
 line numbers in the same file, which have to be updated whenever line numbers
 change due to text addition or deletion.

test/FileCheck/regex-clear.txt

+// RUN: FileCheck -check-prefix X -input-file %s %s
+// RUN: FileCheck -check-prefix Y -input-file %s %s
+// RUN: FileCheck -check-prefixes X,Y -input-file %s %s
+// RUN: FileCheck -check-prefixes X,CY -input-file %s %s
+// RUN: FileCheck -check-prefixes Y,CX -input-file %s %s
+// RUN: not FileCheck -check-prefix CI -input-file %s %s
+// RUN: not FileCheck -check-prefixes X,CA -input-file %s %s
+// RUN: not FileCheck -check-prefixes Y,CA -input-file %s %s
+// RUN: not FileCheck -check-prefixes X,Y,CX -input-file %s %s
+// RUN: not FileCheck -check-prefixes X,Y,CY -input-file %s %s
+
+op x1
+op y2
+op x1
+op y2
+op x1
+op y2
+; X:  [[REGX:x[0-9]]]
+; Y:  [[REGY:y[0-9]]]
+; X:  op [[REGX]]
+; Y:  op [[REGY]]
+; CX:  [[@CLEAR:REGX$]]
+; CY:  [[@CLEAR:REGY$]]
+; CA:  [[@CLEAR:.*]]
+; X:  op [[REGX]]
+; Y:  op [[REGY]]
+
+; Make sure we do fail on invalid regex.
+; CI:  [[@CLEAR:(]]

utils/FileCheck/FileCheck.cpp

class Pattern {
   /// bar at offset 3.
   std::vector<std::pair<StringRef, unsigned>> VariableUses;
 
   /// Maps definitions of variables to their parenthesized capture numbers.
   /// 
   /// E.g. for the pattern "foo[[bar:.*]]baz", VariableDefs will map "bar" to
   /// 1.
   std::map<StringRef, unsigned> VariableDefs;
+  std::map<unsigned, StringRef> ExpressionOperands;
 
   Check::CheckType CheckTy;
 
   /// Contains the number of line this pattern is in.
   unsigned LineNumber;
 
 public:
   explicit Pattern(Check::CheckType Ty) : CheckTy(Ty) {}
 
   /// Returns the location in source code.
   SMLoc getLoc() const { return PatternLoc; }
 
   bool ParsePattern(StringRef PatternStr, StringRef Prefix, SourceMgr &SM,
                     unsigned LineNumber);
-  size_t Match(StringRef Buffer, size_t &MatchLen,
+  size_t Match(const SourceMgr &SM, StringRef Buffer, size_t &MatchLen,
                StringMap<StringRef> &VariableTable) const;
   void PrintFailureInfo(const SourceMgr &SM, StringRef Buffer,
                         const StringMap<StringRef> &VariableTable) const;
 
   bool hasVariable() const {
     return !(VariableUses.empty() && VariableDefs.empty());
   }
 
if (PatternStr.startswith("[[")) {
                         "invalid named regex reference, no ]] found");
         return true;
       }
 
       StringRef MatchStr = PatternStr.substr(2, End);
       PatternStr = PatternStr.substr(End + 4);
 
       // Get the regex name (e.g. "foo").
-      size_t NameEnd = MatchStr.find(':');
+      size_t NameEnd = MatchStr.find_first_of(":+");
       StringRef Name = MatchStr.substr(0, NameEnd);
 
       if (Name.empty()) {
         SM.PrintMessage(SMLoc::getFromPointer(Name.data()), SourceMgr::DK_Error,
                         "invalid name in named regex: empty name");
         return true;
       }
 
-      // Verify that the name/expression is well formed. FileCheck currently
-      // supports @LINE, @LINE+number, @LINE-number expressions. The check here
-      // is relaxed, more strict check is performed in \c EvaluateExpression.
+      // Verify that the name/expression is well formed. FileCheck
+      // currently supports @LINE, @LINE+number, @LINE-number, @CLEAR:regex
+      // expressions. The check here is relaxed, more strict check is
+      // performed in \c EvaluateExpression.
       bool IsExpression = false;
       for (unsigned i = 0, e = Name.size(); i != e; ++i) {
         if (i == 0 && Name[i] == '@') {
-          if (NameEnd != StringRef::npos) {
-            SM.PrintMessage(SMLoc::getFromPointer(Name.data()),
-                            SourceMgr::DK_Error,
-                            "invalid name in named regex definition");
-            return true;
-          }
           IsExpression = true;
           continue;
         }
         if (Name[i] != '_' && !isalnum(Name[i]) &&
             (!IsExpression || (Name[i] != '+' && Name[i] != '-'))) {
           SM.PrintMessage(SMLoc::getFromPointer(Name.data() + i),
                           SourceMgr::DK_Error, "invalid name in named regex");
           return true;
if (PatternStr.startswith("[[")) {
         continue;
       }
 
       // Handle [[foo:.*]].
       VariableDefs[Name] = CurParen;
       RegExStr += '(';
       ++CurParen;
 
-      if (AddRegExToRegEx(MatchStr.substr(NameEnd + 1), CurParen, SM))
+      if (IsExpression) {
+        // Expression inserts always-matching zero-width group and saves
+        // pattern as an operand to be processed when the group match happens.
+        StringRef ExpressionArg = MatchStr.substr(NameEnd + 1);
+        Regex R(ExpressionArg);
+        std::string Error;
+        if (!R.isValid(Error)) {
+          SM.PrintMessage(SMLoc::getFromPointer(ExpressionArg.data()),
+                          SourceMgr::DK_Error, "invalid regex: " + Error);
+          return true;
+        }
+        ExpressionOperands[CurParen - 1] = ExpressionArg;
+      } else if (AddRegExToRegEx(MatchStr.substr(NameEnd + 1), CurParen, SM))
         return true;
 
       RegExStr += ')';
     }
 
     // Handle fixed string matches.
     // Find the end, which is the start of the next regex.
     size_t FixedMatchEnd = PatternStr.find("{{");
else if (Expr[0] != '-')
       return false;
     if (Expr.getAsInteger(10, Offset))
       return false;
   }
   Value = llvm::itostr(LineNumber + Offset);
   return true;
 }
 
+// Clears all variables that match \p VarNameRegEx.
+// Returns true on error.
+static bool ClearVariables(const SourceMgr &SM, StringRef VarNameRegEx,
+                           StringMap<StringRef> &VariableTable) {
+  Regex VarNameRE(VarNameRegEx);
+  std::string Error;
+  if (!VarNameRE.isValid(Error)) {
+    SM.PrintMessage(SMLoc::getFromPointer(VarNameRegEx.data()),
+                    SourceMgr::DK_Error, "invalid regex: " + Error);
+    return true;
+  }
+
+  SmallVector<StringRef, 16> VarsToClear;
+  for (const auto &Var : VariableTable)
+    if (VarNameRE.match(Var.first()))
+      VarsToClear.push_back(Var.first());
+
+  for (const auto &Var : VarsToClear)
+    VariableTable.erase(Var);
+  return false;
+}
+
 /// Matches the pattern string against the input buffer \p Buffer
 ///
 /// This returns the position that is matched or npos if there is no match. If
 /// there is a match, the size of the matched string is returned in \p
 /// MatchLen.
 ///
 /// The \p VariableTable StringMap provides the current values of filecheck
 /// variables and is updated if this match defines new values.
-size_t Pattern::Match(StringRef Buffer, size_t &MatchLen,
+size_t Pattern::Match(const SourceMgr &SM, StringRef Buffer, size_t &MatchLen,
                       StringMap<StringRef> &VariableTable) const {
   // If this is the EOF pattern, match it immediately.
   if (CheckTy == Check::CheckEOF) {
     MatchLen = 0;
     return Buffer.size();
   }
 
   // If this is a fixed string pattern, just match it now.
size_t Pattern::Match(const SourceMgr &SM, StringRef Buffer, size_t &MatchLen,
 
   // Successful regex match.
   assert(!MatchInfo.empty() && "Didn't get any match");
   StringRef FullMatch = MatchInfo[0];
 
   // If this defines any variables, remember their values.
   for (const auto &VariableDef : VariableDefs) {
     assert(VariableDef.second < MatchInfo.size() && "Internal paren error");
+    if (VariableDef.first == "@CLEAR") {
+      ClearVariables(SM, ExpressionOperands.at(VariableDef.second),
+                     VariableTable);
+    } else
       VariableTable[VariableDef.first] = MatchInfo[VariableDef.second];
   }
 
   MatchLen = FullMatch.size();
   return FullMatch.data() - Buffer.data();
 }
 
 
 /// Computes an arbitrary estimate for the quality of matching this pattern at
if (!IsLabelScanMode) {
     // Match "dag strings" (with mixed "not strings" if any).
     LastPos = CheckDag(SM, Buffer, NotStrings, VariableTable);
     if (LastPos == StringRef::npos)
       return StringRef::npos;
   }
 
   // Match itself from the last position after matching CHECK-DAG.
   StringRef MatchBuffer = Buffer.substr(LastPos);
-  size_t MatchPos = Pat.Match(MatchBuffer, MatchLen, VariableTable);
+  size_t MatchPos = Pat.Match(SM, MatchBuffer, MatchLen, VariableTable);
   if (MatchPos == StringRef::npos) {
     PrintCheckFailed(SM, *this, MatchBuffer, VariableTable);
     return StringRef::npos;
   }
 
   // Similar to the above, in "label-scan mode" we can't yet handle CHECK-NEXT
   // or CHECK-NOT
   if (!IsLabelScanMode) {

 /// Verify there's no "not strings" in the given buffer.
 bool CheckString::CheckNot(const SourceMgr &SM, StringRef Buffer,
                            const std::vector<const Pattern *> &NotStrings,
                            StringMap<StringRef> &VariableTable) const {
   for (const Pattern *Pat : NotStrings) {
     assert((Pat->getCheckTy() == Check::CheckNot) && "Expect CHECK-NOT!");
 
     size_t MatchLen = 0;
-    size_t Pos = Pat->Match(Buffer, MatchLen, VariableTable);
+    size_t Pos = Pat->Match(SM, Buffer, MatchLen, VariableTable);
 
     if (Pos == StringRef::npos)
       continue;
 
     SM.PrintMessage(SMLoc::getFromPointer(Buffer.data() + Pos),
                     SourceMgr::DK_Error, Prefix + "-NOT: string occurred!");
     SM.PrintMessage(Pat->getLoc(), SourceMgr::DK_Note,
                     Prefix + "-NOT: pattern specified here");
for (const Pattern &Pat : DagNotStrings) {
     }
 
     assert((Pat.getCheckTy() == Check::CheckDAG) && "Expect CHECK-DAG!");
 
     size_t MatchLen = 0, MatchPos;
 
     // CHECK-DAG always matches from the start.
     StringRef MatchBuffer = Buffer.substr(StartPos);
-    MatchPos = Pat.Match(MatchBuffer, MatchLen, VariableTable);
+    MatchPos = Pat.Match(SM, MatchBuffer, MatchLen, VariableTable);
     // With a group of CHECK-DAGs, a single mismatching means the match on
     // that group of CHECK-DAGs fails immediately.
     if (MatchPos == StringRef::npos) {
       PrintCheckFailed(SM, Pat.getLoc(), Pat, MatchBuffer, VariableTable);
       return StringRef::npos;
     }
     // Re-calc it as the offset relative to the start of the original string.
     MatchPos += StartPos;