[FileCheck] Fix naming of OverflowErrorStr var

[thopre]

Summary

As pointed out by Joel E. Denny in D97845, the OverflowErrorStr variable
is misnamed because the error is raised for any parsing error. Note that
in FileCheck proper this only happens in case of (under|over)flow
because the regex will ensure a number in the correct format is matched.

[jdenny]

Other than a comment request, LGTM.

llvm/lib/FileCheck/FileCheck.cpp

124–129

Could you add a comment somewhere in this function explaining the significance for the FileCheck utility vs. the library? For me, this point has been a source of confusion.

[thopre]

Add comment to explain why the error message does not mention underflow/overflow

llvm/lib/FileCheck/FileCheck.cpp

124–129

Actually this method is only called through the checkInput API of the FileCheck library. The Pattern object is not visible and thus the regex matched is one produced by FileCheck. So even users of the library should not be able to get anything else than an overflow error. I think I purposedly made the error message vague enough to account for potential future use of this method (i.e. the method is more self contained).

I think it would be better to distinguish between overall behavior (only overflow happen) Vs how the method could be called (invalid StrVal). What do you think of the proposed comment?

[jdenny]

llvm/lib/FileCheck/FileCheck.cpp

124–129

It seems fine. Thanks for clarifying.

llvm/lib/FileCheck/FileCheck.cpp

ExpressionFormat::getMatchingString(ExpressionValue IntegerValue) const {
 
   return (Twine(SignPrefix) + AbsoluteValueStr).str();
 }
 
 Expected<ExpressionValue>
 ExpressionFormat::valueFromStringRepr(StringRef StrVal,
                                       const SourceMgr &SM) const {
   bool ValueIsSigned = Value == Kind::Signed;
-  StringRef OverflowErrorStr = "unable to represent numeric value";
+  // 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.
+  StringRef IntegerParseErrorStr = "unable to represent numeric value";
   if (ValueIsSigned) {
     int64_t SignedValue;
 
     if (StrVal.getAsInteger(10, SignedValue))
-      return ErrorDiagnostic::get(SM, StrVal, OverflowErrorStr);
+      return ErrorDiagnostic::get(SM, StrVal, IntegerParseErrorStr);
 
     return ExpressionValue(SignedValue);
   }
 
   bool Hex = Value == Kind::HexUpper || Value == Kind::HexLower;
   uint64_t UnsignedValue;
   if (StrVal.getAsInteger(Hex ? 16 : 10, UnsignedValue))
-    return ErrorDiagnostic::get(SM, StrVal, OverflowErrorStr);
+    return ErrorDiagnostic::get(SM, StrVal, IntegerParseErrorStr);
 
   return ExpressionValue(UnsignedValue);
 }
 
 static int64_t getAsSigned(uint64_t UnsignedValue) {
   // Use memcpy to reinterpret the bitpattern in Value since casting to
   // signed is implementation-defined if the unsigned value is too big to be
   // represented in the signed type and using an union violates type aliasing