[FileCheck] Turn errors into assert in valueFromStringRepr()

getWildcardRegex() guarantees that only valid hex numbers are matched by
FileCheck numeric expressions. This commit therefore only asserts the
lack of parsing failure in valueFromStringRepr().

Depends On D154430

Reviewed By: arichardson

Differential Revision: https://reviews.llvm.org/D154431

Author: RoboTux

llvm/lib/FileCheck/FileCheck.cpp

@@ -134,26 +134,22 @@ static APInt toSigned(APInt AbsVal, bool Negative) {
   return Result;
 }
 
-Expected<APInt>
-ExpressionFormat::valueFromStringRepr(StringRef StrVal,
-                                      const SourceMgr &SM) const {
+APInt ExpressionFormat::valueFromStringRepr(StringRef StrVal,
+                                            const SourceMgr &SM) const {
   bool ValueIsSigned = Value == Kind::Signed;
-  // Both the FileCheck utility and library only call this method with a valid
-  // value in StrVal. This is guaranteed by the regex returned by
-  // getWildcardRegex() above. Only underflow and overflow errors can thus
-  // occur. However new uses of this method could be added in the future so
-  // the error message does not make assumptions about StrVal.
   bool Negative = StrVal.consume_front("-");
   bool Hex = Value == Kind::HexUpper || Value == Kind::HexLower;
   bool MissingFormPrefix =
       !ValueIsSigned && AlternateForm && !StrVal.consume_front("0x");
   (void)MissingFormPrefix;
   assert(!MissingFormPrefix && "missing alternate form prefix");
   APInt ResultValue;
-  bool ParseFailure = StrVal.getAsInteger(Hex ? 16 : 10, ResultValue);
-  if (ParseFailure)
-    return ErrorDiagnostic::get(SM, StrVal,
-                                "unable to represent numeric value");
+  [[maybe_unused]] bool ParseFailure =
+      StrVal.getAsInteger(Hex ? 16 : 10, ResultValue);
+  // Both the FileCheck utility and library only call this method with a valid
+  // value in StrVal. This is guaranteed by the regex returned by
+  // getWildcardRegex() above.
+  assert(!ParseFailure && "unable to represent numeric value");
   return toSigned(ResultValue, Negative);
 }
 
@@ -1176,10 +1172,8 @@ Pattern::MatchResult Pattern::match(StringRef Buffer,
 
     StringRef MatchedValue = MatchInfo[CaptureParenGroup];
     ExpressionFormat Format = DefinedNumericVariable->getImplicitFormat();
-    Expected<APInt> Value = Format.valueFromStringRepr(MatchedValue, SM);
-    if (!Value)
-      return MatchResult(TheMatch, Value.takeError());
-    DefinedNumericVariable->setValue(*Value, MatchedValue);
+    APInt Value = Format.valueFromStringRepr(MatchedValue, SM);
+    DefinedNumericVariable->setValue(Value, MatchedValue);
   }
 
   return MatchResult(TheMatch, Error::success());

llvm/lib/FileCheck/FileCheckImpl.h

@@ -96,11 +96,8 @@ struct ExpressionFormat {
   Expected<std::string> getMatchingString(APInt Value) const;
 
   /// \returns the value corresponding to string representation \p StrVal
-  /// according to the matching format represented by this instance or an error
-  /// with diagnostic against \p SM if \p StrVal does not correspond to a valid
-  /// and representable value.
-  Expected<APInt> valueFromStringRepr(StringRef StrVal,
-                                      const SourceMgr &SM) const;
+  /// according to the matching format represented by this instance.
+  APInt valueFromStringRepr(StringRef StrVal, const SourceMgr &SM) const;
 };
 
 /// Class to represent an overflow error that might result when manipulating a

llvm/unittests/FileCheck/FileCheckTest.cpp

@@ -201,29 +201,16 @@ struct ExpressionFormatParameterisedFixture
     EXPECT_THAT_EXPECTED(MatchingString, Failed());
   }
 
-  Expected<APInt> getValueFromStringReprFailure(StringRef Str) {
-    StringRef BufferizedStr = bufferize(SM, Str);
-    return Format.valueFromStringRepr(BufferizedStr, SM);
-  }
-
   template <class T>
   void checkValueFromStringRepr(StringRef Str, T ExpectedVal) {
-    Expected<APInt> ResultValue = getValueFromStringReprFailure(Str);
-    ASSERT_THAT_EXPECTED(ResultValue, Succeeded())
-        << "Failed to get value from " << Str;
-    ASSERT_EQ(ResultValue->isNegative(), ExpectedVal < 0)
+    StringRef BufferizedStr = bufferize(SM, Str);
+    APInt ResultValue = Format.valueFromStringRepr(BufferizedStr, SM);
+    ASSERT_EQ(ResultValue.isNegative(), ExpectedVal < 0)
         << "Value for " << Str << " is not " << ExpectedVal;
-    if (ResultValue->isNegative())
-      EXPECT_EQ(ResultValue->getSExtValue(), static_cast<int64_t>(ExpectedVal));
+    if (ResultValue.isNegative())
+      EXPECT_EQ(ResultValue.getSExtValue(), static_cast<int64_t>(ExpectedVal));
     else
-      EXPECT_EQ(ResultValue->getZExtValue(),
-                static_cast<uint64_t>(ExpectedVal));
-  }
-
-  void checkValueFromStringReprFailure(
-      StringRef Str, StringRef ErrorStr = "unable to represent numeric value") {
-    Expected<APInt> ResultValue = getValueFromStringReprFailure(Str);
-    expectDiagnosticError(ErrorStr, ResultValue.takeError());
+      EXPECT_EQ(ResultValue.getZExtValue(), static_cast<uint64_t>(ExpectedVal));
   }
 };
 
@@ -321,7 +308,10 @@ TEST_P(ExpressionFormatParameterisedFixture, FormatValueFromStringRepr) {
 
   // Wrong casing is not tested because valueFromStringRepr() relies on
   // StringRef's getAsInteger() which does not allow to restrict casing.
-  checkValueFromStringReprFailure(addBasePrefix("G"));
+
+  // Likewise, wrong letter digit for hex value is not tested because it is
+  // only caught by an assert in FileCheck due to getWildcardRegex()
+  // guaranteeing only valid letter digits are used.
 }
 
 TEST_P(ExpressionFormatParameterisedFixture, FormatBoolOperator) {