[PATCH] D60390: FileCheck [10/12]: Add support for signed numeric values

[Thomas Preud'homme via Phabricator via llvm-commits]

thopre added inline comments.


================
Comment at: llvm/lib/Support/FileCheck.cpp:986
                         [](const ErrorDiagnostic &E) {},
+                        [](const ValueError &E) {
+                          switch (E.getType()) {
----------------
jhenderson wrote:
> As far as I can see, this block is just skipping the error? Why the switch/case?
There's now only an OverflowError without type but FYI the goal was to catch the case of a new type of ValueError in the future that shouldn't be ignored here.


================
Comment at: llvm/lib/Support/FileCheckImpl.h:133
+  std::error_code convertToErrorCode() const override {
+    return inconvertibleErrorCode();
+  }
----------------
jhenderson wrote:
> Perhaps a `not_supported` or `invalid_argument` would make more sense here.
>From llvm/include/llvm/Support/Error.h:

```
/// The value returned by this function can be returned from convertToErrorCode
/// for Error values where no sensible translation to std::error_code exists.
/// It should only be used in this situation, and should never be used where a
/// sensible conversion to std::error_code is available, as attempts to convert
/// to/from this error will result in a fatal error. (i.e. it is a programmatic
///error to try to convert such a value).
std::error_code inconvertibleErrorCode();
```

That seems to be the right function to use when that function does not make sense for a given error?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60390