[llvm] r263749 - [Support] Add ExitOnError utility to support tools that use the exit-on-error
Lang Hames via llvm-commits
llvm-commits at lists.llvm.org
Thu Mar 17 17:22:06 PDT 2016
Hi Dave,
Thanks for the review. I've implemented most of your suggestions in r263764
and r263768.
Is there much need/benefit to being able to set the banner separately,
> rather than just always on construction?
Yes - I expect the idiomatic use of ExitOnError will be a global variable
accessible everywhere in the tool, and initialized in main with the tool's
name:
ExitOnError ExitOnErr;
<Lots of tool code>
int main(int argc, char *argv[0]) {
ExitOnErr.setBanner(std::string(argv[0]) + " error: "));
...
}
Cheers,
Lang.
On Thu, Mar 17, 2016 at 2:45 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
> On Thu, Mar 17, 2016 at 2:28 PM, Lang Hames via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: lhames
>> Date: Thu Mar 17 16:28:49 2016
>> New Revision: 263749
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=263749&view=rev
>> Log:
>> [Support] Add ExitOnError utility to support tools that use the
>> exit-on-error
>> idiom.
>>
>> Most LLVM tool code exits immediately when an error is encountered and
>> prints an
>> error message to stderr. The ExitOnError class supports this by providing
>> two
>> call operators - one for Errors, and one for Expected<T>s. Calls to code
>> that
>> can return Errors (or Expected<T>s) can use these calls to bail out on
>> error,
>> and otherwise continue as if the operation had succeeded. E.g.
>>
>> Error foo();
>> Expected<int> bar();
>>
>> int main(int argc, char *argv[]) {
>> ExitOnError ExitOnErr;
>>
>> ExitOnErr.setBanner(std::string("Error in ") + argv[0] + ":");
>>
>> // Exit if foo returns an error. No need to manually check error return.
>> ExitOnErr(foo());
>>
>> // Exit if bar returns an error, otherwise unwrap the contained int and
>> // continue.
>> int X = ExitOnErr(bar());
>>
>> // ...
>>
>> return 0;
>> }
>>
>>
>> Modified:
>> llvm/trunk/include/llvm/Support/Error.h
>> llvm/trunk/unittests/Support/ErrorTest.cpp
>>
>> Modified: llvm/trunk/include/llvm/Support/Error.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/Error.h?rev=263749&r1=263748&r2=263749&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/include/llvm/Support/Error.h (original)
>> +++ llvm/trunk/include/llvm/Support/Error.h Thu Mar 17 16:28:49 2016
>> @@ -519,7 +519,8 @@ inline void handleAllErrors(Error E) {
>> /// (allowing clean deallocation of resources, etc.), while reporting
>> error
>> /// information to the user.
>> template <typename... HandlerTs>
>> -void logAllUnhandledErrors(Error E, raw_ostream &OS, std::string
>> ErrorBanner) {
>> +void logAllUnhandledErrors(Error E, raw_ostream &OS,
>> + const std::string &ErrorBanner) {
>> if (!E)
>> return;
>> OS << ErrorBanner;
>> @@ -742,6 +743,55 @@ inline std::error_code errorToErrorCode(
>> return EC;
>> }
>>
>> +/// Helper for check-and-exit error handling.
>> +///
>> +/// For tool use only. NOT FOR USE IN LIBRARY CODE.
>> +///
>> +class ExitOnError {
>> +public:
>> +
>> + /// Create an error on exit helper.
>> + ExitOnError(std::string Banner = "", int DefaultErrorExitCode = 1)
>> + : Banner(Banner),
>>
>
> Missing std::move here ^
>
>
>> + GetExitCode([=](const Error&) { return DefaultErrorExitCode; }) {}
>> +
>> + /// Set the banner string for any errors caught by operator().
>>
>
> Is there much need/benefit to being able to set the banner separately,
> rather than just always on construction?
>
>
>> + void setBanner(std::string Banner) {
>> + this->Banner = Banner;
>>
>
> Missing std::move here ^
>
>
>> + }
>
> +
>> + /// Set the exit-code mapper function.
>> + void setExitCodeMapper(std::function<int(const Error&)> GetExitCode) {
>> + this->GetExitCode = GetExitCode;
>>
>
> Missing std::move here ^
>
>
>> + }
>> +
>> + /// Check Err. If it's in a failure state log the error(s) and exit.
>> + void operator()(Error Err) const {
>> + checkError(std::move(Err));
>> + }
>> +
>> + /// Check E. If it's in a success state return the contained value. If
>> it's
>> + /// in a failure state log the error(s) and exit.
>> + template <typename T>
>> + T operator()(Expected<T> E) const {
>> + checkError(E.takeError());
>> + return std::move(*E);
>>
>
> Maybe take E by rvalue ref (since you're not moving it, just bits of it,
> and T might be expensive to copy/non-movable)? Or do you need to be able to
> pass copies/lvalues?
>
>
>> + }
>> +
>> +private:
>> +
>> + void checkError(Error Err) const {
>> + if (Err) {
>> + int ExitCode = GetExitCode(Err);
>> + logAllUnhandledErrors(std::move(Err), errs(), Banner);
>> + exit(ExitCode);
>> + }
>> + }
>> +
>> + std::string Banner;
>> + std::function<int(const Error&)> GetExitCode;
>> +};
>> +
>> } // namespace llvm
>>
>> #endif // LLVM_SUPPORT_ERROR_H
>>
>> Modified: llvm/trunk/unittests/Support/ErrorTest.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/ErrorTest.cpp?rev=263749&r1=263748&r2=263749&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/unittests/Support/ErrorTest.cpp (original)
>> +++ llvm/trunk/unittests/Support/ErrorTest.cpp Thu Mar 17 16:28:49 2016
>> @@ -38,6 +38,7 @@ namespace {
>> // - consume_error to consume a "safe" error without any output.
>> // - handleAllUnhandledErrors to assert that all errors are handled.
>> // - logAllUnhandledErrors to log errors to a stream.
>> +// - ExitOnError tests.
>> //
>> // Expected tests:
>> // - Expected<T> with T.
>> @@ -50,6 +51,7 @@ namespace {
>> // - std::error_code to Error (ECError) in failure mode.
>> // - Error to std::error_code in success mode.
>> // - Error (ECError) to std::error_code in failure mode.
>> +//
>>
>> // Custom error class with a default base class and some random 'info'
>> attached.
>> class CustomError : public ErrorInfo<CustomError> {
>> @@ -376,6 +378,32 @@ TEST(Error, CheckErrorUtilities) {
>> EXPECT_EQ(ErrorInfo, 7)
>> << "Failed to handle Error returned from handleErrors.";
>> }
>> +
>> + // Test ExitOnError
>>
>
> Probably good to pull it out into a separate test - more specific errors
> from gtest/lit, and it means less repetition when running those EXPECT_EXIT
> tests (each one has to be run in a separate process, and execute all the
> preamble in the same test case until tehy reach the unique EXPECT_EXIT
> they're intendting to run - so you might even consider splitting them out
> even further)
>
>
>> + {
>> + ExitOnError ExitOnErr;
>> + ExitOnErr.setBanner("Error in tool:");
>> + ExitOnErr.setExitCodeMapper(
>> + [](const Error &E) {
>> + if (E.isA<CustomSubError>())
>> + return 2;
>> + return 1;
>> + });
>> +
>> + // Make sure we don't bail on success.
>> + ExitOnErr(Error::success());
>> + EXPECT_EQ(ExitOnErr(Expected<int>(7)), 7)
>> + << "exitOnError returned an invalid value for Expected";
>> +
>> + // Exit tests.
>> + EXPECT_EXIT(ExitOnErr(make_error<CustomError>(7)),
>> + ::testing::ExitedWithCode(1), "Error in tool:")
>> + << "exitOnError returned an unexpected error result";
>> +
>> + EXPECT_EXIT(ExitOnErr(Expected<int>(make_error<CustomSubError>(0,
>> 0))),
>> + ::testing::ExitedWithCode(2), "Error in tool:")
>> + << "exitOnError returned an unexpected error result";
>> + }
>> }
>>
>> // Test Expected behavior.
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160317/04af2c42/attachment.html>
More information about the llvm-commits
mailing list