[PATCH] D98342: [FileCheck] Fix naming of OverflowErrorStr var

Thomas Preud'homme via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 10 14:20:57 PST 2021


thopre added inline comments.


================
Comment at: llvm/lib/FileCheck/FileCheck.cpp:124
   bool ValueIsSigned = Value == Kind::Signed;
-  StringRef OverflowErrorStr = "unable to represent numeric value";
+  StringRef IntegerParseErrorStr = "unable to represent numeric value";
   if (ValueIsSigned) {
----------------
jdenny wrote:
> 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.
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?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98342/new/

https://reviews.llvm.org/D98342



More information about the llvm-commits mailing list