[FileCheck] Turn errors into assert in valueFromStringRepr()

[thopre]

Summary

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

[thopre]

Simplify signature since it does not throw any error

llvm/lib/FileCheck/FileCheck.cpp

static APInt toSigned(APInt AbsVal, bool Negative) {
   if (AbsVal.isSignBitSet())
     AbsVal = AbsVal.zext(nextAPIntBitWidth(AbsVal.getBitWidth()));
   APInt Result = AbsVal;
   if (Negative)
     Result.negate();
   return Result;
 }
 
-Expected<APInt>
-ExpressionFormat::valueFromStringRepr(StringRef StrVal,
+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);
 }
 
 Expected<APInt> llvm::exprAdd(const APInt &LeftOperand,
                               const APInt &RightOperand, bool &Overflow) {
   return LeftOperand.sadd_ov(RightOperand, Overflow);
 }
 
const NumericVariableMatch &NumericVariableMatch =
         NumericVariableDef.getValue();
     unsigned CaptureParenGroup = NumericVariableMatch.CaptureParenGroup;
     assert(CaptureParenGroup < MatchInfo.size() && "Internal paren error");
     NumericVariable *DefinedNumericVariable =
         NumericVariableMatch.DefinedNumericVariable;
 
     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());
 }
 
 unsigned Pattern::computeMatchDistance(StringRef Buffer) const {
   // Just compute the number of matching characters. For regular expressions, we
   // just compare against the regex itself and hope for the best.

llvm/lib/FileCheck/FileCheckImpl.h

public:
   Expected<std::string> getWildcardRegex() const;
 
   /// \returns the string representation of \p Value in the format represented
   /// by this instance, or an error if conversion to this format failed or the
   /// format is NoFormat.
   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
 /// value.
 class OverflowError : public ErrorInfo<OverflowError> {
 public:
   static char ID;
 

llvm/unittests/FileCheck/FileCheckTest.cpp

struct ExpressionFormatParameterisedFixture
 
   template <class T> void checkMatchingStringFailure(T Val) {
     APInt Value(LiteralsBitWidth, Val, std::is_signed_v<T>);
     Expected<std::string> MatchingString = Format.getMatchingString(Value);
     // Error message tested in ExpressionFormat unit tests.
     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));
   }
 };
 
 TEST_P(ExpressionFormatParameterisedFixture, FormatGetWildcardRegex) {
   // Wildcard regex is valid.
   Expected<std::string> WildcardPattern = Format.getWildcardRegex();
   ASSERT_THAT_EXPECTED(WildcardPattern, Succeeded());
   WildcardRegex = Regex((Twine("^") + *WildcardPattern + "$").str());
if (Signed) {
     checkValueFromStringRepr(addBasePrefix(MaxUint64Str), MaxUint64);
   }
 
   checkValueFromStringRepr(addBasePrefix(TenStr), 10);
   checkValueFromStringRepr(addBasePrefix(FifteenStr), 15);
 
   // 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) {
   EXPECT_TRUE(bool(Format));
 }
 
 INSTANTIATE_TEST_SUITE_P(
     AllowedExplicitExpressionFormat, ExpressionFormatParameterisedFixture,