FileCheck: Return parse error w/ Error & Expected

[thopre]

Summary

Make use of Error and Expected to bubble up diagnostics and force
checking of errors in the callers.

[arichardson]

Looks good to me. Just two minor comments:

This adds quite a lot of return make_error<ParseErrorInfo>(SM.GetMessage(SMLoc::getFromPointer(Foo.data()), SourceMgr::DK_Error, "...")));
Maybe adding a helper would make sense? Something like:

Error ParseErrorInfo::get(const SourceManager& SM, const char* LocPtr, const Twine& ErrMsg) {
   return make_error<ParseErrorInfo>(SM.GetMessage(SMLoc::getFromPointer(LocPtr), SourceMgr::DK_Error,  ErrMsg)));
}

Then the above can just be return ParseErrorInfo::get(SM, Foo.data(), "...");

[thopre]

• Fix bugs in unit test when running with Error checks enabled
• Make eval() functions return an Expected instead of Optional
• Create helper functions to create ParseError and its associated diagnostic
• Rename ParseErrorInfo into FileCheckParseError

[thopre]

llvm/include/llvm/Support/FileCheck.h

351–352 ↗

(On Diff #204238)

This can be simplified to Diagnostic.print(nullptr, OS);

llvm/lib/Support/FileCheck.cpp

619–622 ↗

(On Diff #204238)

Perhaps worth making match() return an error via an Expected<size_t> return value.

[thopre]

• Convert match to return Expected as well
• Simplify FileCheckErrorDiagnostic logging

[thopre]

Fix unit test

[thopre]

Replace assert(false) by llvm_unreachable

[thopre]

Fix code style issues

[thopre]

Remove "represent value" -> "store value" change

[probinson]

LGTM.

[thopre]

• Remove useless llvm_unreachable from handleAllErrors
• Move comment to apply to both ignored errors

llvm/trunk/include/llvm/Support/FileCheck.h

public:
 
   /// \returns the line number where this variable is defined.
   size_t getDefLineNumber() { return DefLineNumber; }
 };
 
 /// Type of functions evaluating a given binary operation.
 using binop_eval_t = uint64_t (*)(uint64_t, uint64_t);
 
+/// Class to represent an undefined variable error which prints that variable's
+/// name between quotes when printed.
+class FileCheckUndefVarError : public ErrorInfo<FileCheckUndefVarError> {
+private:
+  StringRef VarName;
+
+public:
+  static char ID;
+
+  FileCheckUndefVarError(StringRef VarName) : VarName(VarName) {}
+
+  StringRef getVarName() const { return VarName; }
+
+  std::error_code convertToErrorCode() const override {
+    return inconvertibleErrorCode();
+  }
+
+  /// Print name of variable associated with this error.
+  void log(raw_ostream &OS) const override {
+    OS << "\"";
+    OS.write_escaped(VarName) << "\"";
+  }
+};
+
 /// Class representing a numeric expression consisting of either a single
 /// numeric variable or a binary operation between a numeric variable and an
 /// immediate.
 class FileCheckNumExpr {
 private:
   /// Left operand.
   FileCheckNumericVariable *LeftOp;
 
   /// Right operand.
   uint64_t RightOp;
 
   /// Pointer to function that can evaluate this binary operation.
   binop_eval_t EvalBinop;
 
 public:
   FileCheckNumExpr(binop_eval_t EvalBinop,
                    FileCheckNumericVariable *OperandLeft, uint64_t OperandRight)
       : LeftOp(OperandLeft), RightOp(OperandRight), EvalBinop(EvalBinop) {}
 
   /// Evaluates the value of this numeric expression, using EvalBinop to
-  /// perform the binary operation it consists of. \returns None if the numeric
-  /// variable used is undefined, or the expression value otherwise.
-  Optional<uint64_t> eval() const;
-
-  /// \returns the name of the undefined variable used in this expression if
-  /// any or an empty string otherwise.
-  StringRef getUndefVarName() const;
+  /// perform the binary operation it consists of. \returns an error if the
+  /// numeric variable used is undefined, or the expression value otherwise.
+  Expected<uint64_t> eval() const;
 };
 
 class FileCheckPatternContext;
 
 /// Class representing a substitution to perform in the RegExStr string.
 class FileCheckSubstitution {
 protected:
   /// Pointer to a class instance holding, among other things, the table with
public:
 
   /// \returns the string to be substituted for something else.
   StringRef getFromString() const { return FromStr; }
 
   /// \returns the index where the substitution is to be performed in RegExStr.
   size_t getIndex() const { return InsertIdx; }
 
   /// \returns a string containing the result of the substitution represented
-  /// by this class instance or None if substitution failed.
-  virtual Optional<std::string> getResult() const = 0;
-
-  /// \returns the name of the variable used in this substitution if undefined,
-  /// or an empty string otherwise.
-  virtual StringRef getUndefVarName() const = 0;
+  /// by this class instance or an error if substitution failed.
+  virtual Expected<std::string> getResult() const = 0;
 };
 
 class FileCheckStringSubstitution : public FileCheckSubstitution {
 public:
   FileCheckStringSubstitution(FileCheckPatternContext *Context,
                               StringRef VarName, size_t InsertIdx)
       : FileCheckSubstitution(Context, VarName, InsertIdx) {}
 
   /// \returns the text that the string variable in this substitution matched
-  /// when defined, or None if the variable is undefined.
-  Optional<std::string> getResult() const override;
-
-  /// \returns the name of the string variable used in this substitution if
-  /// undefined, or an empty string otherwise.
-  StringRef getUndefVarName() const override;
+  /// when defined, or an error if the variable is undefined.
+  Expected<std::string> getResult() const override;
 };
 
 class FileCheckNumericSubstitution : public FileCheckSubstitution {
 private:
   /// Pointer to the class representing the numeric expression whose value is
   /// to be substituted.
   FileCheckNumExpr *NumExpr;
 
 public:
   FileCheckNumericSubstitution(FileCheckPatternContext *Context, StringRef Expr,
                                FileCheckNumExpr *NumExpr, size_t InsertIdx)
       : FileCheckSubstitution(Context, Expr, InsertIdx), NumExpr(NumExpr) {}
 
   /// \returns a string containing the result of evaluating the numeric
-  /// expression in this substitution, or None if evaluation failed.
-  Optional<std::string> getResult() const override;
-
-  /// \returns the name of the numeric variable used in this substitution if
-  /// undefined, or an empty string otherwise.
-  StringRef getUndefVarName() const override;
+  /// expression in this substitution, or an error if evaluation failed.
+  Expected<std::string> getResult() const override;
 };
 
 //===----------------------------------------------------------------------===//
 // Pattern handling code.
 //===----------------------------------------------------------------------===//
 
 namespace Check {
 
private:
   /// automatically free them once they are guaranteed to no longer be used.
   std::vector<std::unique_ptr<FileCheckNumericVariable>> NumericVariables;
 
   /// Vector holding pointers to all substitutions. Used to automatically free
   /// them once they are guaranteed to no longer be used.
   std::vector<std::unique_ptr<FileCheckSubstitution>> Substitutions;
 
 public:
-  /// \returns the value of string variable \p VarName or None if no such
+  /// \returns the value of string variable \p VarName or an error if no such
   /// variable has been defined.
-  Optional<StringRef> getPatternVarValue(StringRef VarName);
+  Expected<StringRef> getPatternVarValue(StringRef VarName);
 
   /// Defines string and numeric variables from definitions given on the
   /// command line, passed as a vector of [#]VAR=VAL strings in
-  /// \p CmdlineDefines. Reports any error to \p SM and \returns whether an
-  /// error occured.
-  bool defineCmdlineVariables(std::vector<std::string> &CmdlineDefines,
+  /// \p CmdlineDefines. \returns an error list containing diagnostics against
+  /// \p SM for all definition parsing failures, if any, or Success otherwise.
+  Error defineCmdlineVariables(std::vector<std::string> &CmdlineDefines,
                                SourceMgr &SM);
 
   /// Undefines local variables (variables whose name does not start with a '$'
   /// sign), i.e. removes them from GlobalVariableTable and from
   /// GlobalNumericVariableTable and also clears the value of numeric
   /// variables.
   void clearLocalVars();
 
 private:
private:
 
   /// Makes a new numeric substitution and registers it for destruction when
   /// the context is destroyed.
   FileCheckSubstitution *makeNumericSubstitution(StringRef Expr,
                                                  FileCheckNumExpr *NumExpr,
                                                  size_t InsertIdx);
 };
 
+/// Class to represent an error holding a diagnostic with location information
+/// used when printing it.
+class FileCheckErrorDiagnostic : public ErrorInfo<FileCheckErrorDiagnostic> {
+private:
+  SMDiagnostic Diagnostic;
+
+public:
+  static char ID;
+
+  FileCheckErrorDiagnostic(SMDiagnostic &&Diag) : Diagnostic(Diag) {}
+
+  std::error_code convertToErrorCode() const override {
+    return inconvertibleErrorCode();
+  }
+
+  /// Print diagnostic associated with this error when printing the error.
+  void log(raw_ostream &OS) const override { Diagnostic.print(nullptr, OS); }
+
+  static Error get(const SourceMgr &SM, SMLoc Loc, const Twine &ErrMsg) {
+    return make_error<FileCheckErrorDiagnostic>(
+        SM.GetMessage(Loc, SourceMgr::DK_Error, ErrMsg));
+  }
+
+  static Error get(const SourceMgr &SM, StringRef Buffer, const Twine &ErrMsg) {
+    return get(SM, SMLoc::getFromPointer(Buffer.data()), ErrMsg);
+  }
+};
+
+class FileCheckNotFoundError : public ErrorInfo<FileCheckNotFoundError> {
+public:
+  static char ID;
+
+  std::error_code convertToErrorCode() const override {
+    return inconvertibleErrorCode();
+  }
+
+  /// Print diagnostic associated with this error when printing the error.
+  void log(raw_ostream &OS) const override {
+    OS << "String not found in input";
+  }
+};
+
 class FileCheckPattern {
   SMLoc PatternLoc;
 
   /// A fixed string to match as the pattern or empty if this pattern requires
   /// a regex match.
   StringRef FixedStr;
 
   /// A regex string to match as the pattern or empty if this pattern requires
public:
   SMLoc getLoc() const { return PatternLoc; }
 
   /// \returns the pointer to the global state for all patterns in this
   /// FileCheck instance.
   FileCheckPatternContext *getContext() const { return Context; }
 
   /// \returns whether \p C is a valid first character for a variable name.
   static bool isValidVarNameStart(char C);
-  /// Parses the string at the start of \p Str for a variable name and \returns
-  /// whether the variable name is ill-formed. If parsing succeeded, sets
-  /// \p IsPseudo to indicate if it is a pseudo variable, sets \p Name to the
-  /// parsed variable name and strips \p Str from the variable name.
-  static bool parseVariable(StringRef &Str, StringRef &Name, bool &IsPseudo);
-  /// Parses \p Expr for the definition of a numeric variable, returning an
-  /// error if \p Context already holds a string variable with the same name.
-  /// \returns whether parsing fails, in which case errors are reported on
-  /// \p SM. Otherwise, sets \p Name to the name of the parsed numeric
-  /// variable.
-  static bool parseNumericVariableDefinition(StringRef &Expr, StringRef &Name,
+  /// Parses the string at the start of \p Str for a variable name. \returns
+  /// an error holding a diagnostic against \p SM if parsing fail, or the
+  /// name of the variable otherwise. In the latter case, sets \p IsPseudo to
+  /// indicate if it is a pseudo variable and strips \p Str from the variable
+  /// name.
+  static Expected<StringRef> parseVariable(StringRef &Str, bool &IsPseudo,
+                                           const SourceMgr &SM);
+  /// Parses \p Expr for the name of a numeric variable to be defined. \returns
+  /// an error holding a diagnostic against \p SM should defining such a
+  /// variable be invalid, or Success otherwise. In the latter case, sets
+  /// \p Name to the name of the parsed numeric variable name.
+  static Error parseNumericVariableDefinition(StringRef &Expr, StringRef &Name,
                                               FileCheckPatternContext *Context,
                                               const SourceMgr &SM);
   /// Parses \p Expr for a numeric substitution block. \returns the class
   /// representing the AST of the numeric expression whose value must be
-  /// substituted, or nullptr if parsing fails, in which case errors are
-  /// reported on \p SM. Sets \p DefinedNumericVariable to point to the class
-  /// representing the numeric variable defined in this numeric substitution
-  /// block, or nullptr if this block does not define any variable.
-  FileCheckNumExpr *parseNumericSubstitutionBlock(
-      StringRef Expr, FileCheckNumericVariable *&DefinedNumericVariable,
+  /// substituted, or an error holding a diagnostic against \p SM if parsing
+  /// fails. If substitution was successful, sets \p DefinedNumericVariable to
+  /// point to the class representing the numeric variable defined in this
+  /// numeric substitution block, or None if this block does not define any
+  /// variable.
+  Expected<FileCheckNumExpr *> parseNumericSubstitutionBlock(
+      StringRef Expr,
+      Optional<FileCheckNumericVariable *> &DefinedNumericVariable,
       const SourceMgr &SM) const;
   /// Parses the pattern in \p PatternStr and initializes this FileCheckPattern
   /// instance accordingly.
   ///
   /// \p Prefix provides which prefix is being matched, \p Req describes the
   /// global options that influence the parsing such as whitespace
   /// canonicalization, \p SM provides the SourceMgr used for error reports.
   /// \returns true in case of an error, false otherwise.
   bool parsePattern(StringRef PatternStr, StringRef Prefix, SourceMgr &SM,
                     const FileCheckRequest &Req);
   /// Matches the pattern string against the input buffer \p Buffer
   ///
-  /// \returns the position that is matched or npos if there is no match. If
-  /// there is a match, updates \p MatchLen with the size of the matched
-  /// string.
+  /// \returns the position that is matched or an error indicating why matching
+  /// failed. If there is a match, updates \p MatchLen with the size of the
+  /// matched string.
   ///
   /// The GlobalVariableTable StringMap in the FileCheckPatternContext class
   /// instance provides the current values of FileCheck string variables and
   /// is updated if this match defines new values. Likewise, the
   /// GlobalNumericVariableTable StringMap in the same class provides the
   /// current values of FileCheck numeric variables and is updated if this
   /// match defines new numeric values.
-  size_t match(StringRef Buffer, size_t &MatchLen, const SourceMgr &SM) const;
+  Expected<size_t> match(StringRef Buffer, size_t &MatchLen,
+                         const SourceMgr &SM) const;
   /// Prints the value of successful substitutions or the name of the undefined
   /// string or numeric variable preventing a successful substitution.
   void printSubstitutions(const SourceMgr &SM, StringRef Buffer,
                           SMRange MatchRange = None) const;
   void printFuzzyMatch(const SourceMgr &SM, StringRef Buffer,
                        std::vector<FileCheckDiag> *Diags) const;
 
   bool hasVariable() const {
private:
   ///
   /// \p Str has to point in the beginning of the definition (right after the
   /// opening sequence). \p SM holds the SourceMgr used for error repporting.
   ///  \returns the offset of the closing sequence within Str, or npos if it
   /// was not found.
   size_t FindRegexVarEnd(StringRef Str, SourceMgr &SM);
 
   /// Parses \p Expr for the use of a numeric variable. \returns the pointer to
-  /// the class instance representing that variable if successful, or nullptr
-  /// otherwise, in which case errors are reported on \p SM.
-  FileCheckNumericVariable *parseNumericVariableUse(StringRef &Expr,
-                                                    const SourceMgr &SM) const;
+  /// the class instance representing that variable if successful, or an error
+  /// holding a diagnostic against \p SM otherwise.
+  Expected<FileCheckNumericVariable *>
+  parseNumericVariableUse(StringRef &Expr, const SourceMgr &SM) const;
   /// Parses \p Expr for a binary operation.
   /// \returns the class representing the binary operation of the numeric
-  /// expression, or nullptr if parsing fails, in which case errors are
-  /// reported on \p SM.
-  FileCheckNumExpr *parseBinop(StringRef &Expr, const SourceMgr &SM) const;
+  /// expression, or an error holding a diagnostic against \p SM otherwise.
+  Expected<FileCheckNumExpr *> parseBinop(StringRef &Expr,
+                                          const SourceMgr &SM) const;
 };
 
 //===----------------------------------------------------------------------===//
 /// Summary of a FileCheck diagnostic.
 //===----------------------------------------------------------------------===//
 
 struct FileCheckDiag {
   /// What is the FileCheck directive for this diagnostic?

llvm/trunk/lib/Support/FileCheck.cpp

 
 bool FileCheckNumericVariable::clearValue() {
   if (!Value)
     return true;
   Value = None;
   return false;
 }
 
-Optional<uint64_t> FileCheckNumExpr::eval() const {
+Expected<uint64_t> FileCheckNumExpr::eval() const {
   assert(LeftOp && "Evaluating an empty numeric expression");
   Optional<uint64_t> LeftOpValue = LeftOp->getValue();
   // Variable is undefined.
   if (!LeftOpValue)
-    return None;
+    return make_error<FileCheckUndefVarError>(LeftOp->getName());
   return EvalBinop(*LeftOpValue, RightOp);
 }
 
-StringRef FileCheckNumExpr::getUndefVarName() const {
-  if (!LeftOp->getValue())
-    return LeftOp->getName();
-  return StringRef();
-}
-
-Optional<std::string> FileCheckNumericSubstitution::getResult() const {
-  Optional<uint64_t> EvaluatedValue = NumExpr->eval();
+Expected<std::string> FileCheckNumericSubstitution::getResult() const {
+  Expected<uint64_t> EvaluatedValue = NumExpr->eval();
   if (!EvaluatedValue)
-    return None;
+    return EvaluatedValue.takeError();
   return utostr(*EvaluatedValue);
 }
 
-Optional<std::string> FileCheckStringSubstitution::getResult() const {
+Expected<std::string> FileCheckStringSubstitution::getResult() const {
   // Look up the value and escape it so that we can put it into the regex.
-  Optional<StringRef> VarVal = Context->getPatternVarValue(FromStr);
+  Expected<StringRef> VarVal = Context->getPatternVarValue(FromStr);
   if (!VarVal)
-    return None;
+    return VarVal.takeError();
   return Regex::escape(*VarVal);
 }
 
-StringRef FileCheckNumericSubstitution::getUndefVarName() const {
-  // Although a use of an undefined numeric variable is detected at parse
-  // time, a numeric variable can be undefined later by ClearLocalVariables.
-  return NumExpr->getUndefVarName();
-}
-
-StringRef FileCheckStringSubstitution::getUndefVarName() const {
-  if (!Context->getPatternVarValue(FromStr))
-    return FromStr;
-
-  return StringRef();
-}
-
 bool FileCheckPattern::isValidVarNameStart(char C) {
   return C == '_' || isalpha(C);
 }
 
-bool FileCheckPattern::parseVariable(StringRef &Str, StringRef &Name,
-                                     bool &IsPseudo) {
+Expected<StringRef> FileCheckPattern::parseVariable(StringRef &Str,
+                                                    bool &IsPseudo,
+                                                    const SourceMgr &SM) {
   if (Str.empty())
-    return true;
+    return FileCheckErrorDiagnostic::get(SM, Str, "empty variable name");
 
   bool ParsedOneChar = false;
   unsigned I = 0;
   IsPseudo = Str[0] == '@';
 
   // Global vars start with '$'.
   if (Str[0] == '$' || IsPseudo)
     ++I;
 
   for (unsigned E = Str.size(); I != E; ++I) {
     if (!ParsedOneChar && !isValidVarNameStart(Str[I]))
-      return true;
+      return FileCheckErrorDiagnostic::get(SM, Str, "invalid variable name");
 
     // Variable names are composed of alphanumeric characters and underscores.
     if (Str[I] != '_' && !isalnum(Str[I]))
       break;
     ParsedOneChar = true;
   }
 
-  Name = Str.take_front(I);
+  StringRef Name = Str.take_front(I);
   Str = Str.substr(I);
-  return false;
+  return Name;
 }
 
 // StringRef holding all characters considered as horizontal whitespaces by
 // FileCheck input canonicalization.
 StringRef SpaceChars = " \t";
 
 // Parsing helper function that strips the first character in S and returns it.
 static char popFront(StringRef &S) {
   char C = S.front();
   S = S.drop_front();
   return C;
 }
 
-bool FileCheckPattern::parseNumericVariableDefinition(
+char FileCheckUndefVarError::ID = 0;
+char FileCheckErrorDiagnostic::ID = 0;
+char FileCheckNotFoundError::ID = 0;
+
+Error FileCheckPattern::parseNumericVariableDefinition(
     StringRef &Expr, StringRef &Name, FileCheckPatternContext *Context,
     const SourceMgr &SM) {
   bool IsPseudo;
-  if (parseVariable(Expr, Name, IsPseudo)) {
-    SM.PrintMessage(SMLoc::getFromPointer(Expr.data()), SourceMgr::DK_Error,
-                    "invalid variable name");
-    return true;
-  }
-
-  if (IsPseudo) {
-    SM.PrintMessage(SMLoc::getFromPointer(Name.data()), SourceMgr::DK_Error,
-                    "definition of pseudo numeric variable unsupported");
-    return true;
-  }
+  Expected<StringRef> ParseVarResult = parseVariable(Expr, IsPseudo, SM);
+  if (!ParseVarResult)
+    return ParseVarResult.takeError();
+  Name = *ParseVarResult;
+
+  if (IsPseudo)
+    return FileCheckErrorDiagnostic::get(
+        SM, Name, "definition of pseudo numeric variable unsupported");
 
   // Detect collisions between string and numeric variables when the latter
   // is created later than the former.
   if (Context->DefinedVariableTable.find(Name) !=
-      Context->DefinedVariableTable.end()) {
-    SM.PrintMessage(SMLoc::getFromPointer(Name.data()), SourceMgr::DK_Error,
-                    "string variable with name '" + Name + "' already exists");
-    return true;
-  }
+      Context->DefinedVariableTable.end())
+    return FileCheckErrorDiagnostic::get(
+        SM, Name, "string variable with name '" + Name + "' already exists");
 
-  return false;
+  return Error::success();
 }
 
-FileCheckNumericVariable *
+Expected<FileCheckNumericVariable *>
 FileCheckPattern::parseNumericVariableUse(StringRef &Expr,
                                           const SourceMgr &SM) const {
   bool IsPseudo;
-  StringRef Name;
-  if (parseVariable(Expr, Name, IsPseudo)) {
-    SM.PrintMessage(SMLoc::getFromPointer(Expr.data()), SourceMgr::DK_Error,
-                    "invalid variable name");
-    return nullptr;
-  }
-
-  if (IsPseudo && !Name.equals("@LINE")) {
-    SM.PrintMessage(SMLoc::getFromPointer(Name.data()), SourceMgr::DK_Error,
-                    "invalid pseudo numeric variable '" + Name + "'");
-    return nullptr;
-  }
+  Expected<StringRef> ParseVarResult = parseVariable(Expr, IsPseudo, SM);
+  if (!ParseVarResult)
+    return ParseVarResult.takeError();
+  StringRef Name = *ParseVarResult;
+
+  if (IsPseudo && !Name.equals("@LINE"))
+    return FileCheckErrorDiagnostic::get(
+        SM, Name, "invalid pseudo numeric variable '" + Name + "'");
 
   // This method is indirectly called from parsePattern for all numeric
   // variable definitions and uses in the order in which they appear in the
   // CHECK pattern. For each definition, the pointer to the class instance of
   // the corresponding numeric variable definition is stored in
   // GlobalNumericVariableTable. Therefore, the pointer we get below is for the
   // class instance corresponding to the last definition of this variable use.
   auto VarTableIter = Context->GlobalNumericVariableTable.find(Name);
-  if (VarTableIter == Context->GlobalNumericVariableTable.end()) {
-    SM.PrintMessage(SMLoc::getFromPointer(Name.data()), SourceMgr::DK_Error,
-                    "using undefined numeric variable '" + Name + "'");
-    return nullptr;
-  }
+  if (VarTableIter == Context->GlobalNumericVariableTable.end())
+    return FileCheckErrorDiagnostic::get(
+        SM, Name, "using undefined numeric variable '" + Name + "'");
 
   FileCheckNumericVariable *NumericVariable = VarTableIter->second;
-  if (!IsPseudo && NumericVariable->getDefLineNumber() == LineNumber) {
-    SM.PrintMessage(SMLoc::getFromPointer(Name.data()), SourceMgr::DK_Error,
-                    "numeric variable '" + Name +
-                        "' defined on the same line as used");
-    return nullptr;
-  }
+  if (!IsPseudo && NumericVariable->getDefLineNumber() == LineNumber)
+    return FileCheckErrorDiagnostic::get(
+        SM, Name,
+        "numeric variable '" + Name + "' defined on the same line as used");
 
   return NumericVariable;
 }
 
 static uint64_t add(uint64_t LeftOp, uint64_t RightOp) {
   return LeftOp + RightOp;
 }
 
 static uint64_t sub(uint64_t LeftOp, uint64_t RightOp) {
   return LeftOp - RightOp;
 }
 
-FileCheckNumExpr *FileCheckPattern::parseBinop(StringRef &Expr,
-                                               const SourceMgr &SM) const {
-  FileCheckNumericVariable *LeftOp = parseNumericVariableUse(Expr, SM);
-  if (!LeftOp) {
-    // Error reporting done in parseNumericVariableUse().
-    return nullptr;
+Expected<FileCheckNumExpr *>
+FileCheckPattern::parseBinop(StringRef &Expr, const SourceMgr &SM) const {
+  Expected<FileCheckNumericVariable *> LeftParseResult =
+      parseNumericVariableUse(Expr, SM);
+  if (!LeftParseResult) {
+    return LeftParseResult.takeError();
   }
+  FileCheckNumericVariable *LeftOp = *LeftParseResult;
 
   // Check if this is a supported operation and select a function to perform
   // it.
   Expr = Expr.ltrim(SpaceChars);
   if (Expr.empty())
     return Context->makeNumExpr(add, LeftOp, 0);
   SMLoc OpLoc = SMLoc::getFromPointer(Expr.data());
   char Operator = popFront(Expr);
   binop_eval_t EvalBinop;
   switch (Operator) {
   case '+':
     EvalBinop = add;
     break;
   case '-':
     EvalBinop = sub;
     break;
   default:
-    SM.PrintMessage(OpLoc, SourceMgr::DK_Error,
-                    Twine("unsupported numeric operation '") + Twine(Operator) +
-                        "'");
-    return nullptr;
+    return FileCheckErrorDiagnostic::get(
+        SM, OpLoc,
+        Twine("unsupported numeric operation '") + Twine(Operator) + "'");
   }
 
   // Parse right operand.
   Expr = Expr.ltrim(SpaceChars);
-  if (Expr.empty()) {
-    SM.PrintMessage(SMLoc::getFromPointer(Expr.data()), SourceMgr::DK_Error,
-                    "missing operand in numeric expression");
-    return nullptr;
-  }
+  if (Expr.empty())
+    return FileCheckErrorDiagnostic::get(
+        SM, Expr, "missing operand in numeric expression");
   uint64_t RightOp;
-  if (Expr.consumeInteger(10, RightOp)) {
-    SM.PrintMessage(SMLoc::getFromPointer(Expr.data()), SourceMgr::DK_Error,
-                    "invalid offset in numeric expression '" + Expr + "'");
-    return nullptr;
-  }
+  if (Expr.consumeInteger(10, RightOp))
+    return FileCheckErrorDiagnostic::get(
+        SM, Expr, "invalid offset in numeric expression '" + Expr + "'");
   Expr = Expr.ltrim(SpaceChars);
-  if (!Expr.empty()) {
-    SM.PrintMessage(SMLoc::getFromPointer(Expr.data()), SourceMgr::DK_Error,
-                    "unexpected characters at end of numeric expression '" +
-                        Expr + "'");
-    return nullptr;
-  }
+  if (!Expr.empty())
+    return FileCheckErrorDiagnostic::get(
+        SM, Expr,
+        "unexpected characters at end of numeric expression '" + Expr + "'");
 
   return Context->makeNumExpr(EvalBinop, LeftOp, RightOp);
 }
 
-FileCheckNumExpr *FileCheckPattern::parseNumericSubstitutionBlock(
-    StringRef Expr, FileCheckNumericVariable *&DefinedNumericVariable,
+Expected<FileCheckNumExpr *> FileCheckPattern::parseNumericSubstitutionBlock(
+    StringRef Expr,
+    Optional<FileCheckNumericVariable *> &DefinedNumericVariable,
     const SourceMgr &SM) const {
   // Parse the numeric variable definition.
-  DefinedNumericVariable = nullptr;
+  DefinedNumericVariable = None;
   size_t DefEnd = Expr.find(':');
   if (DefEnd != StringRef::npos) {
     StringRef DefExpr = Expr.substr(0, DefEnd);
     StringRef UseExpr = Expr = Expr.substr(DefEnd + 1);
 
     DefExpr = DefExpr.ltrim(SpaceChars);
     StringRef Name;
-    if (parseNumericVariableDefinition(DefExpr, Name, Context, SM)) {
-      // Invalid variable definition. Error reporting done in parsing function.
-      return nullptr;
-    }
+    Error ErrorDiagnostic =
+        parseNumericVariableDefinition(DefExpr, Name, Context, SM);
+    if (ErrorDiagnostic)
+      return std::move(ErrorDiagnostic);
 
     DefinedNumericVariable =
         Context->makeNumericVariable(this->LineNumber, Name);
 
     DefExpr = DefExpr.ltrim(SpaceChars);
-    if (!DefExpr.empty()) {
-      SM.PrintMessage(SMLoc::getFromPointer(DefExpr.data()),
-                      SourceMgr::DK_Error,
-                      "invalid numeric variable definition");
-      return nullptr;
-    }
+    if (!DefExpr.empty())
+      return FileCheckErrorDiagnostic::get(
+          SM, DefExpr, "invalid numeric variable definition");
     UseExpr = UseExpr.ltrim(SpaceChars);
-    if (!UseExpr.empty()) {
-      SM.PrintMessage(
-          SMLoc::getFromPointer(UseExpr.data()), SourceMgr::DK_Error,
+    if (!UseExpr.empty())
+      return FileCheckErrorDiagnostic::get(
+          SM, UseExpr,
           "unexpected string after variable definition: '" + UseExpr + "'");
-      return nullptr;
-    }
     return Context->makeNumExpr(add, nullptr, 0);
   }
 
   // Parse the numeric expression itself.
   Expr = Expr.ltrim(SpaceChars);
   return parseBinop(Expr, SM);
 }
 
if (PatternStr.startswith("[[")) {
         if (SpacePos != StringRef::npos) {
           SM.PrintMessage(SMLoc::getFromPointer(MatchStr.data() + SpacePos),
                           SourceMgr::DK_Error, "unexpected whitespace");
           return true;
         }
 
         // Get the name (e.g. "foo") and verify it is well formed.
         bool IsPseudo;
-        StringRef Name;
         StringRef OrigMatchStr = MatchStr;
-        if (parseVariable(MatchStr, Name, IsPseudo)) {
-          SM.PrintMessage(SMLoc::getFromPointer(MatchStr.data()),
-                          SourceMgr::DK_Error, "invalid variable name");
+        Expected<StringRef> ParseVarResult =
+            parseVariable(MatchStr, IsPseudo, SM);
+        if (!ParseVarResult) {
+          logAllUnhandledErrors(ParseVarResult.takeError(), errs());
           return true;
         }
+        StringRef Name = *ParseVarResult;
 
         IsDefinition = (VarEndIdx != StringRef::npos);
         if (IsDefinition) {
           if ((IsPseudo || !MatchStr.consume_front(":"))) {
             SM.PrintMessage(SMLoc::getFromPointer(Name.data()),
                             SourceMgr::DK_Error,
                             "invalid name in string variable definition");
             return true;
if (PatternStr.startswith("[[")) {
             IsNumBlock = true;
           } else
             SubstStr = Name;
         }
       }
 
       // Parse numeric substitution block.
       FileCheckNumExpr *NumExpr;
-      FileCheckNumericVariable *DefinedNumericVariable;
+      Optional<FileCheckNumericVariable *> DefinedNumericVariable;
       if (IsNumBlock) {
-        NumExpr =
+        Expected<FileCheckNumExpr *> ParseResult =
             parseNumericSubstitutionBlock(MatchStr, DefinedNumericVariable, SM);
-        if (NumExpr == nullptr)
+        if (!ParseResult) {
+          logAllUnhandledErrors(ParseResult.takeError(), errs());
           return true;
+        }
+        NumExpr = *ParseResult;
         if (DefinedNumericVariable) {
           IsDefinition = true;
-          DefName = DefinedNumericVariable->getName();
+          DefName = (*DefinedNumericVariable)->getName();
           MatchRegexp = StringRef("[0-9]+");
         } else
           SubstStr = MatchStr;
       }
 
       // Handle substitutions: [[foo]] and [[#<foo expr>]].
       if (!IsDefinition) {
         // Handle substitution of string variables that were defined earlier on
if (PatternStr.startswith("[[")) {
           Substitutions.push_back(Substitution);
         }
         continue;
       }
 
       // Handle variable definitions: [[<def>:(...)]] and
       // [[#(...)<def>:(...)]].
       if (IsNumBlock) {
-        FileCheckNumExprMatch NumExprDef = {DefinedNumericVariable, CurParen};
+        FileCheckNumExprMatch NumExprDef = {*DefinedNumericVariable, CurParen};
         NumericVariableDefs[DefName] = NumExprDef;
         // This store is done here rather than in match() to allow
         // parseNumericVariableUse() to get the pointer to the class instance
         // of the right variable definition corresponding to a given numeric
         // variable use.
-        Context->GlobalNumericVariableTable[DefName] = DefinedNumericVariable;
+        Context->GlobalNumericVariableTable[DefName] = *DefinedNumericVariable;
       } else {
         VariableDefs[DefName] = CurParen;
         // Mark the string variable as defined to detect collisions between
         // string and numeric variables in parseNumericVariableUse() and
         // DefineCmdlineVariables() when the latter is created later than the
         // former. We cannot reuse GlobalVariableTable for this by populating
         // it with an empty string since we would then lose the ability to
         // detect the use of an undefined variable in match().

 }
 
 void FileCheckPattern::AddBackrefToRegEx(unsigned BackrefNum) {
   assert(BackrefNum >= 1 && BackrefNum <= 9 && "Invalid backref number");
   std::string Backref = std::string("\\") + std::string(1, '0' + BackrefNum);
   RegExStr += Backref;
 }
 
-size_t FileCheckPattern::match(StringRef Buffer, size_t &MatchLen,
+Expected<size_t> FileCheckPattern::match(StringRef Buffer, size_t &MatchLen,
                                          const SourceMgr &SM) 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.
   if (!FixedStr.empty()) {
     MatchLen = FixedStr.size();
-    return Buffer.find(FixedStr);
+    size_t Pos = 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
   // actual value.
   StringRef RegExToMatch = RegExStr;
   std::string TmpStr;
   if (!Substitutions.empty()) {
     TmpStr = RegExStr;
 
     size_t InsertOffset = 0;
     // Substitute all string variables and numeric expressions whose values are
     // only now known. Use of string variables defined on the same line are
     // handled by back-references.
     for (const auto &Substitution : Substitutions) {
       // Substitute and check for failure (e.g. use of undefined variable).
-      Optional<std::string> Value = Substitution->getResult();
+      Expected<std::string> Value = Substitution->getResult();
       if (!Value)
-        return StringRef::npos;
+        return Value.takeError();
 
       // Plop it into the regex at the adjusted offset.
       TmpStr.insert(TmpStr.begin() + Substitution->getIndex() + InsertOffset,
                     Value->begin(), Value->end());
       InsertOffset += Value->size();
     }
 
     // Match the newly constructed regex.
     RegExToMatch = TmpStr;
   }
 
   SmallVector<StringRef, 4> MatchInfo;
   if (!Regex(RegExToMatch, Regex::Newline).match(Buffer, &MatchInfo))
-    return StringRef::npos;
+    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) {
     assert(VariableDef.second < MatchInfo.size() && "Internal paren error");
     Context->GlobalVariableTable[VariableDef.first] =
         MatchInfo[VariableDef.second];
   }
 
   // If this defines any numeric variables, remember their values.
   for (const auto &NumericVariableDef : NumericVariableDefs) {
     const FileCheckNumExprMatch &NumericVariableMatch =
         NumericVariableDef.getValue();
     unsigned CaptureParenGroup = NumericVariableMatch.CaptureParenGroup;
     assert(CaptureParenGroup < MatchInfo.size() && "Internal paren error");
     FileCheckNumericVariable *DefinedNumericVariable =
         NumericVariableMatch.DefinedNumericVariable;
 
     StringRef MatchedValue = MatchInfo[CaptureParenGroup];
     uint64_t Val;
-    if (MatchedValue.getAsInteger(10, Val)) {
-      SM.PrintMessage(SMLoc::getFromPointer(MatchedValue.data()),
-                      SourceMgr::DK_Error, "Unable to represent numeric value");
-    }
+    if (MatchedValue.getAsInteger(10, Val))
+      return FileCheckErrorDiagnostic::get(SM, MatchedValue,
+                                           "Unable to represent numeric value");
     if (DefinedNumericVariable->setValue(Val))
-      assert(false && "Numeric variable redefined");
+      llvm_unreachable("Numeric variable redefined");
   }
 
   // Like CHECK-NEXT, CHECK-EMPTY's match range is considered to start after
   // the required preceding newline, which is consumed by the pattern in the
   // case of CHECK-EMPTY but not CHECK-NEXT.
   size_t MatchStartSkip = CheckTy == Check::CheckEmpty;
   MatchLen = FullMatch.size() - MatchStartSkip;
   return FullMatch.data() - Buffer.data() + MatchStartSkip;

 
 void FileCheckPattern::printSubstitutions(const SourceMgr &SM, StringRef Buffer,
                                           SMRange MatchRange) const {
   // Print what we know about substitutions.
   if (!Substitutions.empty()) {
     for (const auto &Substitution : Substitutions) {
       SmallString<256> Msg;
       raw_svector_ostream OS(Msg);
-      Optional<std::string> MatchedValue = Substitution->getResult();
+      Expected<std::string> MatchedValue = Substitution->getResult();
 
       // Substitution failed or is not known at match time, print the undefined
       // variable it uses.
       if (!MatchedValue) {
-        StringRef UndefVarName = Substitution->getUndefVarName();
-        if (UndefVarName.empty())
-          continue;
-        OS << "uses undefined variable \"";
-        OS.write_escaped(UndefVarName) << "\"";
+        bool UndefSeen = false;
+        handleAllErrors(MatchedValue.takeError(),
+                        [](const FileCheckNotFoundError &E) {},
+                        // Handled in PrintNoMatch()
+                        [](const FileCheckErrorDiagnostic &E) {},
+                        [&](const FileCheckUndefVarError &E) {
+                          if (!UndefSeen) {
+                            OS << "uses undefined variable ";
+                            UndefSeen = true;
+                          }
+                          E.log(OS);
+                        },
+                        [](const ErrorInfoBase &E) {
+                          llvm_unreachable("Unexpected error");
+                        });
       } else {
         // Substitution succeeded. Print substituted value.
         OS << "with \"";
         OS.write_escaped(Substitution->getFromString()) << "\" equal to \"";
         OS.write_escaped(*MatchedValue) << "\"";
       }
 
       if (MatchRange.isValid())
if (Best && Best != StringRef::npos && BestQuality < 50) {
     SM.PrintMessage(MatchRange.Start, SourceMgr::DK_Note,
                     "possible intended match here");
 
     // FIXME: If we wanted to be really friendly we would show why the match
     // failed, as it can be hard to spot simple one character differences.
   }
 }
 
-Optional<StringRef>
+Expected<StringRef>
 FileCheckPatternContext::getPatternVarValue(StringRef VarName) {
   auto VarIter = GlobalVariableTable.find(VarName);
   if (VarIter == GlobalVariableTable.end())
-    return None;
+    return make_error<FileCheckUndefVarError>(VarName);
 
   return VarIter->second;
 }
 
 FileCheckNumExpr *
 FileCheckPatternContext::makeNumExpr(binop_eval_t EvalBinop,
                                      FileCheckNumericVariable *OperandLeft,
                                      uint64_t OperandRight) {
FindFirstMatchingPrefix(Regex &PrefixRE, StringRef &Buffer,
   }
 
   // We ran out of buffer while skipping partial matches so give up.
   return {StringRef(), StringRef()};
 }
 
 bool FileCheck::ReadCheckFile(SourceMgr &SM, StringRef Buffer, Regex &PrefixRE,
                               std::vector<FileCheckString> &CheckStrings) {
-  if (PatternContext.defineCmdlineVariables(Req.GlobalDefines, SM))
+  Error DefineError =
+      PatternContext.defineCmdlineVariables(Req.GlobalDefines, SM);
+  if (DefineError) {
+    logAllUnhandledErrors(std::move(DefineError), errs());
     return true;
+  }
 
   std::vector<FileCheckPattern> ImplicitNegativeChecks;
   for (const auto &PatternString : Req.ImplicitCheckNot) {
     // Create a buffer with fake command line content in order to display the
     // command line option responsible for the specific implicit CHECK-NOT.
     std::string Prefix = "-implicit-check-not='";
     std::string Suffix = "'";
     std::unique_ptr<MemoryBuffer> CmdLine = MemoryBuffer::getMemBufferCopy(
static void PrintMatch(bool ExpectedMatch, const SourceMgr &SM,
   PrintMatch(ExpectedMatch, SM, CheckStr.Prefix, CheckStr.Loc, CheckStr.Pat,
              MatchedCount, Buffer, MatchPos, MatchLen, Req, Diags);
 }
 
 static void PrintNoMatch(bool ExpectedMatch, const SourceMgr &SM,
                          StringRef Prefix, SMLoc Loc,
                          const FileCheckPattern &Pat, int MatchedCount,
                          StringRef Buffer, bool VerboseVerbose,
-                         std::vector<FileCheckDiag> *Diags) {
+                         std::vector<FileCheckDiag> *Diags, Error MatchErrors) {
+  assert(MatchErrors && "Called on successful match");
   bool PrintDiag = true;
   if (!ExpectedMatch) {
-    if (!VerboseVerbose)
+    if (!VerboseVerbose) {
+      consumeError(std::move(MatchErrors));
       return;
+    }
     // Due to their verbosity, we don't print verbose diagnostics here if we're
     // gathering them for a different rendering, but we always print other
     // diagnostics.
     PrintDiag = !Diags;
   }
 
   // If the current position is at the end of a line, advance to the start of
   // the next line.
   Buffer = Buffer.substr(Buffer.find_first_not_of(" \t\n\r"));
   SMRange SearchRange = ProcessMatchResult(
       ExpectedMatch ? FileCheckDiag::MatchNoneButExpected
                     : FileCheckDiag::MatchNoneAndExcluded,
       SM, Loc, Pat.getCheckTy(), Buffer, 0, Buffer.size(), Diags);
-  if (!PrintDiag)
+  if (!PrintDiag) {
+    consumeError(std::move(MatchErrors));
+    return;
+  }
+
+  MatchErrors =
+      handleErrors(std::move(MatchErrors),
+                   [](const FileCheckErrorDiagnostic &E) { E.log(errs()); });
+
+  // No problem matching the string per se.
+  if (!MatchErrors)
     return;
+  consumeError(std::move(MatchErrors));
 
   // Print "not found" diagnostic.
   std::string Message = formatv("{0}: {1} string not found in input",
                                 Pat.getCheckTy().getDescription(Prefix),
                                 (ExpectedMatch ? "expected" : "excluded"))
                             .str();
   if (Pat.getCount() > 1)
     Message += formatv(" ({0} out of {1})", MatchedCount, Pat.getCount()).str();
   SM.PrintMessage(
       Loc, ExpectedMatch ? SourceMgr::DK_Error : SourceMgr::DK_Remark, Message);
 
   // Print the "scanning from here" line.
   SM.PrintMessage(SearchRange.Start, SourceMgr::DK_Note, "scanning from here");
 
   // Allow the pattern to print additional information if desired.
   Pat.printSubstitutions(SM, Buffer);
 
   if (ExpectedMatch)
     Pat.printFuzzyMatch(SM, Buffer, Diags);
 }
 
 static void PrintNoMatch(bool ExpectedMatch, const SourceMgr &SM,
                          const FileCheckString &CheckStr, int MatchedCount,
                          StringRef Buffer, bool VerboseVerbose,
-                         std::vector<FileCheckDiag> *Diags) {
+                         std::vector<FileCheckDiag> *Diags, Error MatchErrors) {
   PrintNoMatch(ExpectedMatch, SM, CheckStr.Prefix, CheckStr.Loc, CheckStr.Pat,
-               MatchedCount, Buffer, VerboseVerbose, Diags);
+               MatchedCount, Buffer, VerboseVerbose, Diags,
+               std::move(MatchErrors));
 }
 
 /// Counts the number of newlines in the specified range.
 static unsigned CountNumNewlinesBetween(StringRef Range,
                                         const char *&FirstNewLine) {
   unsigned NumNewLines = 0;
   while (1) {
     // Scan for newline.
size_t FileCheckString::Check(const SourceMgr &SM, StringRef Buffer,
   size_t FirstMatchPos = 0;
   // Go match the pattern Count times. Majority of patterns only match with
   // count 1 though.
   assert(Pat.getCount() != 0 && "pattern count can not be zero");
   for (int i = 1; i <= Pat.getCount(); i++) {
     StringRef MatchBuffer = Buffer.substr(LastMatchEnd);
     size_t CurrentMatchLen;
     // get a match at current start point
-    size_t MatchPos = Pat.match(MatchBuffer, CurrentMatchLen, SM);
-    if (i == 1)
-      FirstMatchPos = LastPos + MatchPos;
+    Expected<size_t> MatchResult = Pat.match(MatchBuffer, CurrentMatchLen, SM);
 
     // report
-    if (MatchPos == StringRef::npos) {
-      PrintNoMatch(true, SM, *this, i, MatchBuffer, Req.VerboseVerbose, Diags);
+    if (!MatchResult) {
+      PrintNoMatch(true, SM, *this, i, MatchBuffer, Req.VerboseVerbose, Diags,
+                   MatchResult.takeError());
       return StringRef::npos;
     }
+    size_t MatchPos = *MatchResult;
     PrintMatch(true, SM, *this, i, MatchBuffer, MatchPos, CurrentMatchLen, Req,
                Diags);
+    if (i == 1)
+      FirstMatchPos = LastPos + MatchPos;
 
     // move start point after the match
     LastMatchEnd += MatchPos + CurrentMatchLen;
   }
   // Full match len counts from first match pos.
   MatchLen = LastMatchEnd - FirstMatchPos;
 
   // Similar to the above, in "label-scan mode" we can't yet handle CHECK-NEXT

 bool FileCheckString::CheckNot(
     const SourceMgr &SM, StringRef Buffer,
     const std::vector<const FileCheckPattern *> &NotStrings,
     const FileCheckRequest &Req, std::vector<FileCheckDiag> *Diags) const {
   for (const FileCheckPattern *Pat : NotStrings) {
     assert((Pat->getCheckTy() == Check::CheckNot) && "Expect CHECK-NOT!");
 
     size_t MatchLen = 0;
-    size_t Pos = Pat->match(Buffer, MatchLen, SM);
+    Expected<size_t> MatchResult = Pat->match(Buffer, MatchLen, SM);
 
-    if (Pos == StringRef::npos) {
+    if (!MatchResult) {
       PrintNoMatch(false, SM, Prefix, Pat->getLoc(), *Pat, 1, Buffer,
-                   Req.VerboseVerbose, Diags);
+                   Req.VerboseVerbose, Diags, MatchResult.takeError());
       continue;
     }
+    size_t Pos = *MatchResult;
 
     PrintMatch(false, SM, Prefix, Pat->getLoc(), *Pat, 1, Buffer, Pos, MatchLen,
                Req, Diags);
 
     return true;
   }
 
   return false;
for (auto PatItr = DagNotStrings.begin(), PatEnd = DagNotStrings.end();
 
     // CHECK-DAG always matches from the start.
     size_t MatchLen = 0, MatchPos = StartPos;
 
     // Search for a match that doesn't overlap a previous match in this
     // CHECK-DAG group.
     for (auto MI = MatchRanges.begin(), ME = MatchRanges.end(); true; ++MI) {
       StringRef MatchBuffer = Buffer.substr(MatchPos);
-      size_t MatchPosBuf = Pat.match(MatchBuffer, MatchLen, SM);
+      Expected<size_t> MatchResult = Pat.match(MatchBuffer, MatchLen, SM);
       // With a group of CHECK-DAGs, a single mismatching means the match on
       // that group of CHECK-DAGs fails immediately.
-      if (MatchPosBuf == StringRef::npos) {
+      if (!MatchResult) {
         PrintNoMatch(true, SM, Prefix, Pat.getLoc(), Pat, 1, MatchBuffer,
-                     Req.VerboseVerbose, Diags);
+                     Req.VerboseVerbose, Diags, MatchResult.takeError());
         return StringRef::npos;
       }
+      size_t MatchPosBuf = *MatchResult;
       // Re-calc it as the offset relative to the start of the original string.
       MatchPos += MatchPosBuf;
       if (Req.VerboseVerbose)
         PrintMatch(true, SM, Prefix, Pat.getLoc(), Pat, 1, Buffer, MatchPos,
                    MatchLen, Req, Diags);
       MatchRange M{MatchPos, MatchPos + MatchLen};
       if (Req.AllowDeprecatedDagOverlap) {
         // We don't need to track all matches in this mode, so we just maintain
if (Prefix != Req.CheckPrefixes.front())
       PrefixRegexStr.push_back('|');
 
     PrefixRegexStr.append(Prefix);
   }
 
   return Regex(PrefixRegexStr);
 }
 
-bool FileCheckPatternContext::defineCmdlineVariables(
+Error FileCheckPatternContext::defineCmdlineVariables(
     std::vector<std::string> &CmdlineDefines, SourceMgr &SM) {
   assert(GlobalVariableTable.empty() && GlobalNumericVariableTable.empty() &&
          "Overriding defined variable with command-line variable definitions");
 
   if (CmdlineDefines.empty())
-    return false;
+    return Error::success();
 
   // Create a string representing the vector of command-line definitions. Each
   // definition is on its own line and prefixed with a definition number to
   // clarify which definition a given diagnostic corresponds to.
   unsigned I = 0;
-  bool ErrorFound = false;
+  Error Errs = Error::success();
   std::string CmdlineDefsDiag;
   StringRef Prefix1 = "Global define #";
   StringRef Prefix2 = ": ";
   for (StringRef CmdlineDef : CmdlineDefines)
     CmdlineDefsDiag +=
         (Prefix1 + Twine(++I) + Prefix2 + CmdlineDef + "\n").str();
 
   // Create a buffer with fake command line content in order to display
   // parsing diagnostic with location information and point to the
   // global definition with invalid syntax.
   std::unique_ptr<MemoryBuffer> CmdLineDefsDiagBuffer =
       MemoryBuffer::getMemBufferCopy(CmdlineDefsDiag, "Global defines");
   StringRef CmdlineDefsDiagRef = CmdLineDefsDiagBuffer->getBuffer();
   SM.AddNewSourceBuffer(std::move(CmdLineDefsDiagBuffer), SMLoc());
 
   SmallVector<StringRef, 4> CmdlineDefsDiagVec;
   CmdlineDefsDiagRef.split(CmdlineDefsDiagVec, '\n', -1 /*MaxSplit*/,
                            false /*KeepEmpty*/);
   for (StringRef CmdlineDefDiag : CmdlineDefsDiagVec) {
     unsigned DefStart = CmdlineDefDiag.find(Prefix2) + Prefix2.size();
     StringRef CmdlineDef = CmdlineDefDiag.substr(DefStart);
     size_t EqIdx = CmdlineDef.find('=');
     if (EqIdx == StringRef::npos) {
-      SM.PrintMessage(SMLoc::getFromPointer(CmdlineDef.data()),
-                      SourceMgr::DK_Error,
-                      "Missing equal sign in global definition");
-      ErrorFound = true;
+      Errs = joinErrors(
+          std::move(Errs),
+          FileCheckErrorDiagnostic::get(
+              SM, CmdlineDef, "missing equal sign in global definition"));
       continue;
     }
 
     // Numeric variable definition.
     if (CmdlineDef[0] == '#') {
       StringRef CmdlineName = CmdlineDef.substr(1, EqIdx - 1);
       StringRef VarName;
       SMLoc CmdlineNameLoc = SMLoc::getFromPointer(CmdlineName.data());
-      bool ParseError = FileCheckPattern::parseNumericVariableDefinition(
+      Error ErrorDiagnostic = FileCheckPattern::parseNumericVariableDefinition(
           CmdlineName, VarName, this, SM);
-      // Check that CmdlineName starts with a valid numeric variable to be
-      // defined and that it is not followed that anything. That second check
-      // detects cases like "FOO+2" in a "FOO+2=10" definition.
-      if (ParseError || !CmdlineName.empty()) {
-        if (!ParseError)
-          SM.PrintMessage(CmdlineNameLoc, SourceMgr::DK_Error,
-                          "invalid variable name");
-        ErrorFound = true;
+      if (ErrorDiagnostic) {
+        Errs = joinErrors(std::move(Errs), std::move(ErrorDiagnostic));
+        continue;
+      }
+      // Check that CmdlineName is only composed of the parsed numeric
+      // variable. This catches cases like "FOO+2" in a "FOO+2=10" definition.
+      if (!CmdlineName.empty()) {
+        Errs = joinErrors(std::move(Errs),
+                          FileCheckErrorDiagnostic::get(
+                              SM, CmdlineNameLoc, "invalid variable name"));
         continue;
       }
 
       // Detect collisions between string and numeric variables when the latter
       // is created later than the former.
       if (DefinedVariableTable.find(VarName) != DefinedVariableTable.end()) {
-        SM.PrintMessage(
-            SMLoc::getFromPointer(VarName.data()), SourceMgr::DK_Error,
-            "string variable with name '" + VarName + "' already exists");
-        ErrorFound = true;
+        Errs = joinErrors(
+            std::move(Errs),
+            FileCheckErrorDiagnostic::get(SM, VarName,
+                                          "string variable with name '" +
+                                              VarName + "' already exists"));
         continue;
       }
 
       StringRef CmdlineVal = CmdlineDef.substr(EqIdx + 1);
       uint64_t Val;
       if (CmdlineVal.getAsInteger(10, Val)) {
-        SM.PrintMessage(SMLoc::getFromPointer(CmdlineVal.data()),
-                        SourceMgr::DK_Error,
+        Errs = joinErrors(std::move(Errs),
+                          FileCheckErrorDiagnostic::get(
+                              SM, CmdlineVal,
                               "invalid value in numeric variable definition '" +
-                            CmdlineVal + "'");
-        ErrorFound = true;
+                                  CmdlineVal + "'"));
         continue;
       }
       auto DefinedNumericVariable = makeNumericVariable(0, VarName);
       DefinedNumericVariable->setValue(Val);
 
       // Record this variable definition.
       GlobalNumericVariableTable[DefinedNumericVariable->getName()] =
           DefinedNumericVariable;
     } else {
       // String variable definition.
       std::pair<StringRef, StringRef> CmdlineNameVal = CmdlineDef.split('=');
       StringRef CmdlineName = CmdlineNameVal.first;
       StringRef OrigCmdlineName = CmdlineName;
-      StringRef Name;
       bool IsPseudo;
-      if (FileCheckPattern::parseVariable(CmdlineName, Name, IsPseudo) ||
-          IsPseudo || !CmdlineName.empty()) {
-        SM.PrintMessage(SMLoc::getFromPointer(OrigCmdlineName.data()),
-                        SourceMgr::DK_Error,
+      Expected<StringRef> ParseVarResult =
+          FileCheckPattern::parseVariable(CmdlineName, IsPseudo, SM);
+      if (!ParseVarResult) {
+        Errs = joinErrors(std::move(Errs), ParseVarResult.takeError());
+        continue;
+      }
+      // Check that CmdlineName does not denote a pseudo variable is only
+      // composed of the parsed numeric variable. This catches cases like
+      // "FOO+2" in a "FOO+2=10" definition.
+      if (IsPseudo || !CmdlineName.empty()) {
+        Errs = joinErrors(std::move(Errs),
+                          FileCheckErrorDiagnostic::get(
+                              SM, OrigCmdlineName,
                               "invalid name in string variable definition '" +
-                            OrigCmdlineName + "'");
-        ErrorFound = true;
+                                  OrigCmdlineName + "'"));
         continue;
       }
+      StringRef Name = *ParseVarResult;
 
       // Detect collisions between string and numeric variables when the former
       // is created later than the latter.
       if (GlobalNumericVariableTable.find(Name) !=
           GlobalNumericVariableTable.end()) {
-        SM.PrintMessage(SMLoc::getFromPointer(Name.data()), SourceMgr::DK_Error,
-                        "numeric variable with name '" + Name +
-                            "' already exists");
-        ErrorFound = true;
+        Errs = joinErrors(std::move(Errs), FileCheckErrorDiagnostic::get(
+                                               SM, Name,
+                                               "numeric variable with name '" +
+                                                   Name + "' already exists"));
         continue;
       }
       GlobalVariableTable.insert(CmdlineNameVal);
       // Mark the string variable as defined to detect collisions between
       // string and numeric variables in DefineCmdlineVariables when the latter
       // is created later than the former. We cannot reuse GlobalVariableTable
       // for this by populating it with an empty string since we would then
       // lose the ability to detect the use of an undefined variable in
       // match().
       DefinedVariableTable[Name] = true;
     }
   }
 
-  return ErrorFound;
+  return Errs;
 }
 
 void FileCheckPatternContext::clearLocalVars() {
   SmallVector<StringRef, 16> LocalPatternVars, LocalNumericVars;
   for (const StringMapEntry<StringRef> &Var : GlobalVariableTable)
     if (Var.first()[0] != '$')
       LocalPatternVars.push_back(Var.first());
 

llvm/trunk/test/FileCheck/numeric-expression.txt

 CLI-STR-CONFLICT-NEXT: Global define #2: NUMVAR=foobar
 CLI-STR-CONFLICT-NEXT: {{^                  \^$}}
 INPUT-NUM-CONFLICT: numeric-expression.txt:[[#@LINE-7]]:22: error: string variable with name 'STRVAR' already exists
 INPUT-NUM-CONFLICT-NEXT: CONFLICT4: redef2 {{\[\[#STRVAR:\]\]}}
 INPUT-NUM-CONFLICT-NEXT: {{^                     \^$}}
 CLI-NUM-CONFLICT: Global defines:2:20: error: string variable with name 'STRVAR' already exists
 CLI-NUM-CONFLICT-NEXT: Global define #2: #STRVAR=42
 CLI-NUM-CONFLICT-NEXT: {{^                   \^$}}
+
+; Numeric variable definition with too big value.
+RUN: not FileCheck --check-prefix BIGVAL --input-file %s %s 2>&1 \
+RUN:   | FileCheck --strict-whitespace --check-prefix BIGVAL-MSG %s
+
+BIG VALUE
+NUMVAR: 10000000000000000000000
+BIGVAL-LABEL: BIG VALUE
+BIGVAL-NEXT: NUMVAR: [[#NUMVAR:]]
+BIGVAL-MSG: numeric-expression.txt:[[#@LINE-3]]:9: error: Unable to represent numeric value
+BIGVAL-MSG-NEXT: {{N}}UMVAR: 10000000000000000000000
+BIGVAL-MSG-NEXT: {{^        \^$}}

llvm/trunk/test/FileCheck/string-defines-diagnostics.txt

 RUN:   | FileCheck %s --check-prefix ERRCLIVAR2
 
 ERRCLIVAR2: Missing variable name in command-line definition '-D='
 
 ; Invalid variable name: starts with a digit.
 RUN: not FileCheck -D10VALUE=10 --input-file %s %s 2>&1 \
 RUN:   | FileCheck %s --strict-whitespace --check-prefix ERRCLIFMT
 
-ERRCLIFMT: Global defines:1:19: error: invalid name in string variable definition '10VALUE'
+ERRCLIFMT: Global defines:1:19: error: invalid variable name
 ERRCLIFMT-NEXT: Global define #1: 10VALUE=10
 ERRCLIFMT-NEXT: {{^                  \^$}}
 
 ; Invalid definition of pseudo variable.
 RUN: not FileCheck -D@VALUE=10 --input-file %s %s 2>&1 \
 RUN:   | FileCheck %s --strict-whitespace --check-prefix ERRCLIPSEUDO
 
 ERRCLIPSEUDO: Global defines:1:19: error: invalid name in string variable definition '@VALUE'

llvm/trunk/unittests/Support/FileCheckTest.cpp

TEST_F(FileCheckTest, NumericVariable) {
   EXPECT_FALSE(FooVar.clearValue());
   Value = FooVar.getValue();
   EXPECT_FALSE(Value);
   EXPECT_TRUE(FooVar.clearValue());
 }
 
 uint64_t doAdd(uint64_t OpL, uint64_t OpR) { return OpL + OpR; }
 
+static void expectUndefError(const Twine &ExpectedStr, Error Err) {
+  handleAllErrors(std::move(Err), [&](const FileCheckUndefVarError &E) {
+    EXPECT_EQ(ExpectedStr.str(), E.getVarName());
+  });
+}
+
 TEST_F(FileCheckTest, NumExpr) {
   FileCheckNumericVariable FooVar = FileCheckNumericVariable("FOO", 42);
   FileCheckNumExpr NumExpr = FileCheckNumExpr(doAdd, &FooVar, 18);
 
-  // Defined variable: eval returns right value, no undefined variable
-  // returned.
-  llvm::Optional<uint64_t> Value = NumExpr.eval();
-  EXPECT_TRUE(Value);
+  // Defined variable: eval returns right value.
+  Expected<uint64_t> Value = NumExpr.eval();
+  EXPECT_TRUE(static_cast<bool>(Value));
   EXPECT_EQ(60U, *Value);
-  StringRef UndefVar = NumExpr.getUndefVarName();
-  EXPECT_EQ("", UndefVar);
 
   // Undefined variable: eval fails, undefined variable returned. We call
   // getUndefVarName first to check that it can be called without calling
   // eval() first.
   FooVar.clearValue();
-  UndefVar = NumExpr.getUndefVarName();
-  EXPECT_EQ("FOO", UndefVar);
-  Value = NumExpr.eval();
-  EXPECT_FALSE(Value);
+  Error EvalError = NumExpr.eval().takeError();
+  EXPECT_TRUE(errorToBool(std::move(EvalError)));
+  expectUndefError("FOO", std::move(EvalError));
 }
 
 TEST_F(FileCheckTest, ValidVarNameStart) {
   EXPECT_TRUE(FileCheckPattern::isValidVarNameStart('a'));
   EXPECT_TRUE(FileCheckPattern::isValidVarNameStart('G'));
   EXPECT_TRUE(FileCheckPattern::isValidVarNameStart('_'));
   EXPECT_FALSE(FileCheckPattern::isValidVarNameStart('2'));
   EXPECT_FALSE(FileCheckPattern::isValidVarNameStart('$'));
   EXPECT_FALSE(FileCheckPattern::isValidVarNameStart('@'));
   EXPECT_FALSE(FileCheckPattern::isValidVarNameStart('+'));
   EXPECT_FALSE(FileCheckPattern::isValidVarNameStart('-'));
   EXPECT_FALSE(FileCheckPattern::isValidVarNameStart(':'));
 }
 
+static StringRef bufferize(SourceMgr &SM, StringRef Str) {
+  std::unique_ptr<MemoryBuffer> Buffer =
+      MemoryBuffer::getMemBufferCopy(Str, "TestBuffer");
+  StringRef StrBufferRef = Buffer->getBuffer();
+  SM.AddNewSourceBuffer(std::move(Buffer), SMLoc());
+  return StrBufferRef;
+}
+
 TEST_F(FileCheckTest, ParseVar) {
-  StringRef OrigVarName = "GoodVar42";
+  SourceMgr SM;
+  StringRef OrigVarName = bufferize(SM, "GoodVar42");
   StringRef VarName = OrigVarName;
-  StringRef ParsedName;
   bool IsPseudo = true;
-  EXPECT_FALSE(FileCheckPattern::parseVariable(VarName, ParsedName, IsPseudo));
-  EXPECT_EQ(ParsedName, OrigVarName);
+  Expected<StringRef> ParsedName =
+      FileCheckPattern::parseVariable(VarName, IsPseudo, SM);
+  EXPECT_TRUE(static_cast<bool>(ParsedName));
+  EXPECT_EQ(*ParsedName, OrigVarName);
   EXPECT_TRUE(VarName.empty());
   EXPECT_FALSE(IsPseudo);
 
-  VarName = OrigVarName = "$GoodGlobalVar";
+  VarName = OrigVarName = bufferize(SM, "$GoodGlobalVar");
   IsPseudo = true;
-  EXPECT_FALSE(FileCheckPattern::parseVariable(VarName, ParsedName, IsPseudo));
-  EXPECT_EQ(ParsedName, OrigVarName);
+  ParsedName = FileCheckPattern::parseVariable(VarName, IsPseudo, SM);
+  EXPECT_TRUE(static_cast<bool>(ParsedName));
+  EXPECT_EQ(*ParsedName, OrigVarName);
   EXPECT_TRUE(VarName.empty());
   EXPECT_FALSE(IsPseudo);
 
-  VarName = OrigVarName = "@GoodPseudoVar";
+  VarName = OrigVarName = bufferize(SM, "@GoodPseudoVar");
   IsPseudo = true;
-  EXPECT_FALSE(FileCheckPattern::parseVariable(VarName, ParsedName, IsPseudo));
-  EXPECT_EQ(ParsedName, OrigVarName);
+  ParsedName = FileCheckPattern::parseVariable(VarName, IsPseudo, SM);
+  EXPECT_TRUE(static_cast<bool>(ParsedName));
+  EXPECT_EQ(*ParsedName, OrigVarName);
   EXPECT_TRUE(VarName.empty());
   EXPECT_TRUE(IsPseudo);
 
-  VarName = "42BadVar";
-  EXPECT_TRUE(FileCheckPattern::parseVariable(VarName, ParsedName, IsPseudo));
+  VarName = bufferize(SM, "42BadVar");
+  ParsedName = FileCheckPattern::parseVariable(VarName, IsPseudo, SM);
+  EXPECT_TRUE(errorToBool(ParsedName.takeError()));
+
+  VarName = bufferize(SM, "$@");
+  ParsedName = FileCheckPattern::parseVariable(VarName, IsPseudo, SM);
+  EXPECT_TRUE(errorToBool(ParsedName.takeError()));
 
-  VarName = "$@";
-  EXPECT_TRUE(FileCheckPattern::parseVariable(VarName, ParsedName, IsPseudo));
-
-  VarName = OrigVarName = "B@dVar";
+  VarName = OrigVarName = bufferize(SM, "B@dVar");
   IsPseudo = true;
-  EXPECT_FALSE(FileCheckPattern::parseVariable(VarName, ParsedName, IsPseudo));
+  ParsedName = FileCheckPattern::parseVariable(VarName, IsPseudo, SM);
+  EXPECT_TRUE(static_cast<bool>(ParsedName));
   EXPECT_EQ(VarName, OrigVarName.substr(1));
-  EXPECT_EQ(ParsedName, "B");
+  EXPECT_EQ(*ParsedName, "B");
   EXPECT_FALSE(IsPseudo);
 
-  VarName = OrigVarName = "B$dVar";
+  VarName = OrigVarName = bufferize(SM, "B$dVar");
   IsPseudo = true;
-  EXPECT_FALSE(FileCheckPattern::parseVariable(VarName, ParsedName, IsPseudo));
+  ParsedName = FileCheckPattern::parseVariable(VarName, IsPseudo, SM);
+  EXPECT_TRUE(static_cast<bool>(ParsedName));
   EXPECT_EQ(VarName, OrigVarName.substr(1));
-  EXPECT_EQ(ParsedName, "B");
+  EXPECT_EQ(*ParsedName, "B");
   EXPECT_FALSE(IsPseudo);
 
-  VarName = "BadVar+";
+  VarName = bufferize(SM, "BadVar+");
   IsPseudo = true;
-  EXPECT_FALSE(FileCheckPattern::parseVariable(VarName, ParsedName, IsPseudo));
+  ParsedName = FileCheckPattern::parseVariable(VarName, IsPseudo, SM);
+  EXPECT_TRUE(static_cast<bool>(ParsedName));
   EXPECT_EQ(VarName, "+");
-  EXPECT_EQ(ParsedName, "BadVar");
+  EXPECT_EQ(*ParsedName, "BadVar");
   EXPECT_FALSE(IsPseudo);
 
-  VarName = "BadVar-";
+  VarName = bufferize(SM, "BadVar-");
   IsPseudo = true;
-  EXPECT_FALSE(FileCheckPattern::parseVariable(VarName, ParsedName, IsPseudo));
+  ParsedName = FileCheckPattern::parseVariable(VarName, IsPseudo, SM);
+  EXPECT_TRUE(static_cast<bool>(ParsedName));
   EXPECT_EQ(VarName, "-");
-  EXPECT_EQ(ParsedName, "BadVar");
+  EXPECT_EQ(*ParsedName, "BadVar");
   EXPECT_FALSE(IsPseudo);
 
-  VarName = "BadVar:";
+  VarName = bufferize(SM, "BadVar:");
   IsPseudo = true;
-  EXPECT_FALSE(FileCheckPattern::parseVariable(VarName, ParsedName, IsPseudo));
+  ParsedName = FileCheckPattern::parseVariable(VarName, IsPseudo, SM);
+  EXPECT_TRUE(static_cast<bool>(ParsedName));
   EXPECT_EQ(VarName, ":");
-  EXPECT_EQ(ParsedName, "BadVar");
+  EXPECT_EQ(*ParsedName, "BadVar");
   EXPECT_FALSE(IsPseudo);
 }
 
-static StringRef bufferize(SourceMgr &SM, StringRef Str) {
-  std::unique_ptr<MemoryBuffer> Buffer =
-      MemoryBuffer::getMemBufferCopy(Str, "TestBuffer");
-  StringRef StrBufferRef = Buffer->getBuffer();
-  SM.AddNewSourceBuffer(std::move(Buffer), SMLoc());
-  return StrBufferRef;
-}
-
 class PatternTester {
 private:
   size_t LineNumber = 1;
   SourceMgr SM;
   FileCheckRequest Req;
   FileCheckPatternContext Context;
   FileCheckPattern P =
       FileCheckPattern(Check::CheckPlain, &Context, LineNumber++);
 
 public:
   PatternTester() {
     std::vector<std::string> GlobalDefines;
     GlobalDefines.emplace_back(std::string("#FOO=42"));
     GlobalDefines.emplace_back(std::string("BAR=BAZ"));
-    Context.defineCmdlineVariables(GlobalDefines, SM);
+    EXPECT_FALSE(
+        errorToBool(Context.defineCmdlineVariables(GlobalDefines, SM)));
     // Call parsePattern to have @LINE defined.
     P.parsePattern("N/A", "CHECK", SM, Req);
     // parsePattern does not expect to be called twice for the same line and
     // will set FixedStr and RegExStr incorrectly if it is. Therefore prepare
     // a pattern for a different line.
     initNextPattern();
   }
 
   void initNextPattern() {
     P = FileCheckPattern(Check::CheckPlain, &Context, LineNumber++);
   }
 
   bool parseNumVarDefExpect(StringRef Expr) {
     StringRef ExprBufferRef = bufferize(SM, Expr);
     StringRef Name;
-    return FileCheckPattern::parseNumericVariableDefinition(ExprBufferRef, Name,
-                                                            &Context, SM);
+    return errorToBool(FileCheckPattern::parseNumericVariableDefinition(
+        ExprBufferRef, Name, &Context, SM));
   }
 
   bool parseSubstExpect(StringRef Expr) {
     StringRef ExprBufferRef = bufferize(SM, Expr);
-    FileCheckNumericVariable *DefinedNumericVariable;
-    return P.parseNumericSubstitutionBlock(
-               ExprBufferRef, DefinedNumericVariable, SM) == nullptr;
+    Optional<FileCheckNumericVariable *> DefinedNumericVariable;
+    return errorToBool(P.parseNumericSubstitutionBlock(
+                            ExprBufferRef, DefinedNumericVariable, SM)
+                           .takeError());
   }
 
   bool parsePatternExpect(StringRef Pattern) {
     StringRef PatBufferRef = bufferize(SM, Pattern);
     return P.parsePattern(PatBufferRef, "CHECK", SM, Req);
   }
 
   bool matchExpect(StringRef Buffer) {
     StringRef BufferRef = bufferize(SM, Buffer);
     size_t MatchLen;
-    return P.match(BufferRef, MatchLen, SM);
+    return errorToBool(P.match(BufferRef, MatchLen, SM).takeError());
   }
 };
 
 TEST_F(FileCheckTest, ParseNumericVariableDefinition) {
   PatternTester Tester;
 
   // Invalid definition of pseudo.
   EXPECT_TRUE(Tester.parseNumVarDefExpect("@LINE"));
TEST_F(FileCheckTest, Match) {
   EXPECT_FALSE(Tester.matchExpect("18 20"));
 }
 
 TEST_F(FileCheckTest, Substitution) {
   SourceMgr SM;
   FileCheckPatternContext Context;
   std::vector<std::string> GlobalDefines;
   GlobalDefines.emplace_back(std::string("FOO=BAR"));
-  Context.defineCmdlineVariables(GlobalDefines, SM);
+  EXPECT_FALSE(errorToBool(Context.defineCmdlineVariables(GlobalDefines, SM)));
 
-  // Substitution of an undefined string variable fails.
+  // Substitution of an undefined string variable fails and error holds that
+  // variable's name.
   FileCheckStringSubstitution StringSubstitution =
       FileCheckStringSubstitution(&Context, "VAR404", 42);
-  EXPECT_FALSE(StringSubstitution.getResult());
+  Expected<std::string> SubstValue = StringSubstitution.getResult();
+  EXPECT_FALSE(static_cast<bool>(SubstValue));
+  expectUndefError("VAR404", SubstValue.takeError());
 
   // Substitutions of defined pseudo and non-pseudo numeric variables return
   // the right value.
   FileCheckNumericVariable LineVar = FileCheckNumericVariable("@LINE", 42);
   FileCheckNumericVariable NVar = FileCheckNumericVariable("N", 10);
   FileCheckNumExpr NumExprLine = FileCheckNumExpr(doAdd, &LineVar, 0);
   FileCheckNumExpr NumExprN = FileCheckNumExpr(doAdd, &NVar, 3);
   FileCheckNumericSubstitution SubstitutionLine =
       FileCheckNumericSubstitution(&Context, "@LINE", &NumExprLine, 12);
   FileCheckNumericSubstitution SubstitutionN =
       FileCheckNumericSubstitution(&Context, "N", &NumExprN, 30);
-  llvm::Optional<std::string> Value = SubstitutionLine.getResult();
-  EXPECT_TRUE(Value);
+  Expected<std::string> Value = SubstitutionLine.getResult();
+  EXPECT_TRUE(static_cast<bool>(Value));
   EXPECT_EQ("42", *Value);
   Value = SubstitutionN.getResult();
-  EXPECT_TRUE(Value);
+  EXPECT_TRUE(static_cast<bool>(Value));
   EXPECT_EQ("13", *Value);
 
   // Substitution of an undefined numeric variable fails.
   LineVar.clearValue();
-  EXPECT_FALSE(SubstitutionLine.getResult());
+  SubstValue = SubstitutionLine.getResult().takeError();
+  EXPECT_FALSE(static_cast<bool>(SubstValue));
+  expectUndefError("@LINE", SubstValue.takeError());
   NVar.clearValue();
-  EXPECT_FALSE(SubstitutionN.getResult());
+  SubstValue = SubstitutionN.getResult().takeError();
+  EXPECT_FALSE(static_cast<bool>(SubstValue));
+  expectUndefError("N", SubstValue.takeError());
 
   // Substitution of a defined string variable returns the right value.
   FileCheckPattern P = FileCheckPattern(Check::CheckPlain, &Context, 1);
   StringSubstitution = FileCheckStringSubstitution(&Context, "FOO", 42);
   Value = StringSubstitution.getResult();
-  EXPECT_TRUE(Value);
+  EXPECT_TRUE(static_cast<bool>(Value));
   EXPECT_EQ("BAR", *Value);
 }
 
-TEST_F(FileCheckTest, UndefVars) {
-  SourceMgr SM;
-  FileCheckPatternContext Context;
-  std::vector<std::string> GlobalDefines;
-  GlobalDefines.emplace_back(std::string("FOO=BAR"));
-  Context.defineCmdlineVariables(GlobalDefines, SM);
-
-  // getUndefVarName() on a string substitution with an undefined variable
-  // returns that variable.
-  FileCheckStringSubstitution StringSubstitution =
-      FileCheckStringSubstitution(&Context, "VAR404", 42);
-  StringRef UndefVar = StringSubstitution.getUndefVarName();
-  EXPECT_EQ("VAR404", UndefVar);
-
-  // getUndefVarName() on a string substitution with a defined variable returns
-  // an empty string.
-  StringSubstitution = FileCheckStringSubstitution(&Context, "FOO", 42);
-  UndefVar = StringSubstitution.getUndefVarName();
-  EXPECT_EQ("", UndefVar);
-
-  // getUndefVarName() on a numeric substitution with a defined variable
-  // returns an empty string.
-  FileCheckNumericVariable LineVar = FileCheckNumericVariable("@LINE", 42);
-  FileCheckNumExpr NumExpr = FileCheckNumExpr(doAdd, &LineVar, 0);
-  FileCheckNumericSubstitution NumericSubstitution =
-      FileCheckNumericSubstitution(&Context, "@LINE", &NumExpr, 12);
-  UndefVar = NumericSubstitution.getUndefVarName();
-  EXPECT_EQ("", UndefVar);
-
-  // getUndefVarName() on a numeric substitution with an undefined variable
-  // returns that variable.
-  LineVar.clearValue();
-  UndefVar = NumericSubstitution.getUndefVarName();
-  EXPECT_EQ("@LINE", UndefVar);
-}
-
 TEST_F(FileCheckTest, FileCheckContext) {
   FileCheckPatternContext Cxt = FileCheckPatternContext();
   std::vector<std::string> GlobalDefines;
   SourceMgr SM;
 
   // Missing equal sign.
   GlobalDefines.emplace_back(std::string("LocalVar"));
-  EXPECT_TRUE(Cxt.defineCmdlineVariables(GlobalDefines, SM));
+  EXPECT_TRUE(errorToBool(Cxt.defineCmdlineVariables(GlobalDefines, SM)));
   GlobalDefines.clear();
   GlobalDefines.emplace_back(std::string("#LocalNumVar"));
-  EXPECT_TRUE(Cxt.defineCmdlineVariables(GlobalDefines, SM));
+  EXPECT_TRUE(errorToBool(Cxt.defineCmdlineVariables(GlobalDefines, SM)));
 
   // Empty variable name.
   GlobalDefines.clear();
   GlobalDefines.emplace_back(std::string("=18"));
-  EXPECT_TRUE(Cxt.defineCmdlineVariables(GlobalDefines, SM));
+  EXPECT_TRUE(errorToBool(Cxt.defineCmdlineVariables(GlobalDefines, SM)));
   GlobalDefines.clear();
   GlobalDefines.emplace_back(std::string("#=18"));
-  EXPECT_TRUE(Cxt.defineCmdlineVariables(GlobalDefines, SM));
+  EXPECT_TRUE(errorToBool(Cxt.defineCmdlineVariables(GlobalDefines, SM)));
 
   // Invalid variable name.
   GlobalDefines.clear();
   GlobalDefines.emplace_back(std::string("18LocalVar=18"));
-  EXPECT_TRUE(Cxt.defineCmdlineVariables(GlobalDefines, SM));
+  EXPECT_TRUE(errorToBool(Cxt.defineCmdlineVariables(GlobalDefines, SM)));
   GlobalDefines.clear();
   GlobalDefines.emplace_back(std::string("#18LocalNumVar=18"));
-  EXPECT_TRUE(Cxt.defineCmdlineVariables(GlobalDefines, SM));
+  EXPECT_TRUE(errorToBool(Cxt.defineCmdlineVariables(GlobalDefines, SM)));
 
   // Name conflict between pattern and numeric variable.
   GlobalDefines.clear();
   GlobalDefines.emplace_back(std::string("LocalVar=18"));
   GlobalDefines.emplace_back(std::string("#LocalVar=36"));
-  EXPECT_TRUE(Cxt.defineCmdlineVariables(GlobalDefines, SM));
+  EXPECT_TRUE(errorToBool(Cxt.defineCmdlineVariables(GlobalDefines, SM)));
   Cxt = FileCheckPatternContext();
   GlobalDefines.clear();
   GlobalDefines.emplace_back(std::string("#LocalNumVar=18"));
   GlobalDefines.emplace_back(std::string("LocalNumVar=36"));
-  EXPECT_TRUE(Cxt.defineCmdlineVariables(GlobalDefines, SM));
+  EXPECT_TRUE(errorToBool(Cxt.defineCmdlineVariables(GlobalDefines, SM)));
   Cxt = FileCheckPatternContext();
 
   // Invalid numeric value for numeric variable.
   GlobalDefines.clear();
   GlobalDefines.emplace_back(std::string("#LocalNumVar=x"));
-  EXPECT_TRUE(Cxt.defineCmdlineVariables(GlobalDefines, SM));
+  EXPECT_TRUE(errorToBool(Cxt.defineCmdlineVariables(GlobalDefines, SM)));
 
   // Define local variables from command-line.
   GlobalDefines.clear();
   GlobalDefines.emplace_back(std::string("LocalVar=FOO"));
   GlobalDefines.emplace_back(std::string("EmptyVar="));
   GlobalDefines.emplace_back(std::string("#LocalNumVar=18"));
-  bool GotError = Cxt.defineCmdlineVariables(GlobalDefines, SM);
-  EXPECT_FALSE(GotError);
+  EXPECT_FALSE(errorToBool(Cxt.defineCmdlineVariables(GlobalDefines, SM)));
 
   // Check defined variables are present and undefined is absent.
   StringRef LocalVarStr = "LocalVar";
   StringRef LocalNumVarRef = bufferize(SM, "LocalNumVar");
   StringRef EmptyVarStr = "EmptyVar";
   StringRef UnknownVarStr = "UnknownVar";
-  llvm::Optional<StringRef> LocalVar = Cxt.getPatternVarValue(LocalVarStr);
+  Expected<StringRef> LocalVar = Cxt.getPatternVarValue(LocalVarStr);
   FileCheckPattern P = FileCheckPattern(Check::CheckPlain, &Cxt, 1);
-  FileCheckNumericVariable *DefinedNumericVariable;
-  FileCheckNumExpr *NumExpr = P.parseNumericSubstitutionBlock(
+  Optional<FileCheckNumericVariable *> DefinedNumericVariable;
+  Expected<FileCheckNumExpr *> NumExpr = P.parseNumericSubstitutionBlock(
       LocalNumVarRef, DefinedNumericVariable, SM);
-  llvm::Optional<StringRef> EmptyVar = Cxt.getPatternVarValue(EmptyVarStr);
-  llvm::Optional<StringRef> UnknownVar = Cxt.getPatternVarValue(UnknownVarStr);
-  EXPECT_TRUE(LocalVar);
+  Expected<StringRef> EmptyVar = Cxt.getPatternVarValue(EmptyVarStr);
+  Expected<StringRef> UnknownVar = Cxt.getPatternVarValue(UnknownVarStr);
+  EXPECT_TRUE(static_cast<bool>(LocalVar));
   EXPECT_EQ(*LocalVar, "FOO");
-  EXPECT_TRUE(NumExpr);
-  llvm::Optional<uint64_t> NumExprVal = NumExpr->eval();
-  EXPECT_TRUE(NumExprVal);
+  EXPECT_TRUE(static_cast<bool>(NumExpr));
+  Expected<uint64_t> NumExprVal = (*NumExpr)->eval();
+  EXPECT_TRUE(static_cast<bool>(NumExprVal));
   EXPECT_EQ(*NumExprVal, 18U);
-  EXPECT_TRUE(EmptyVar);
+  EXPECT_TRUE(static_cast<bool>(EmptyVar));
   EXPECT_EQ(*EmptyVar, "");
-  EXPECT_FALSE(UnknownVar);
+  EXPECT_TRUE(errorToBool(UnknownVar.takeError()));
 
   // Clear local variables and check they become absent.
   Cxt.clearLocalVars();
   LocalVar = Cxt.getPatternVarValue(LocalVarStr);
-  EXPECT_FALSE(LocalVar);
+  EXPECT_TRUE(errorToBool(LocalVar.takeError()));
   // Check a numeric expression's evaluation fails if called after clearing of
   // local variables, if it was created before. This is important because local
   // variable clearing due to --enable-var-scope happens after numeric
   // expressions are linked to the numeric variables they use.
-  EXPECT_FALSE(NumExpr->eval());
+  EXPECT_TRUE(errorToBool((*NumExpr)->eval().takeError()));
   P = FileCheckPattern(Check::CheckPlain, &Cxt, 2);
   NumExpr = P.parseNumericSubstitutionBlock(LocalNumVarRef,
                                             DefinedNumericVariable, SM);
-  EXPECT_FALSE(NumExpr);
+  EXPECT_TRUE(errorToBool(NumExpr.takeError()));
   EmptyVar = Cxt.getPatternVarValue(EmptyVarStr);
-  EXPECT_FALSE(EmptyVar);
+  EXPECT_TRUE(errorToBool(EmptyVar.takeError()));
 
   // Redefine global variables and check variables are defined again.
   GlobalDefines.emplace_back(std::string("$GlobalVar=BAR"));
   GlobalDefines.emplace_back(std::string("#$GlobalNumVar=36"));
-  GotError = Cxt.defineCmdlineVariables(GlobalDefines, SM);
-  EXPECT_FALSE(GotError);
+  EXPECT_FALSE(errorToBool(Cxt.defineCmdlineVariables(GlobalDefines, SM)));
   StringRef GlobalVarStr = "$GlobalVar";
   StringRef GlobalNumVarRef = bufferize(SM, "$GlobalNumVar");
-  llvm::Optional<StringRef> GlobalVar = Cxt.getPatternVarValue(GlobalVarStr);
-  EXPECT_TRUE(GlobalVar);
+  Expected<StringRef> GlobalVar = Cxt.getPatternVarValue(GlobalVarStr);
+  EXPECT_TRUE(static_cast<bool>(GlobalVar));
   EXPECT_EQ(*GlobalVar, "BAR");
   P = FileCheckPattern(Check::CheckPlain, &Cxt, 3);
   NumExpr = P.parseNumericSubstitutionBlock(GlobalNumVarRef,
                                             DefinedNumericVariable, SM);
-  EXPECT_TRUE(NumExpr);
-  NumExprVal = NumExpr->eval();
-  EXPECT_TRUE(NumExprVal);
+  EXPECT_TRUE(static_cast<bool>(NumExpr));
+  NumExprVal = (*NumExpr)->eval();
+  EXPECT_TRUE(static_cast<bool>(NumExprVal));
   EXPECT_EQ(*NumExprVal, 36U);
 
   // Clear local variables and check global variables remain defined.
   Cxt.clearLocalVars();
-  GlobalVar = Cxt.getPatternVarValue(GlobalVarStr);
-  EXPECT_TRUE(GlobalVar);
+  EXPECT_FALSE(errorToBool(Cxt.getPatternVarValue(GlobalVarStr).takeError()));
   P = FileCheckPattern(Check::CheckPlain, &Cxt, 4);
   NumExpr = P.parseNumericSubstitutionBlock(GlobalNumVarRef,
                                             DefinedNumericVariable, SM);
-  EXPECT_TRUE(NumExpr);
-  NumExprVal = NumExpr->eval();
-  EXPECT_TRUE(NumExprVal);
+  EXPECT_TRUE(static_cast<bool>(NumExpr));
+  NumExprVal = (*NumExpr)->eval();
+  EXPECT_TRUE(static_cast<bool>(NumExprVal));
   EXPECT_EQ(*NumExprVal, 36U);
 }
 } // namespace