[llvm] r264479 - [Support] Switch to RAII helper for error-as-out-parameter idiom.
David Blaikie via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 25 17:12:39 PDT 2016
On Fri, Mar 25, 2016 at 4:54 PM, Lang Hames via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
> Author: lhames
> Date: Fri Mar 25 18:54:32 2016
> New Revision: 264479
>
> URL: http://llvm.org/viewvc/llvm-project?rev=264479&view=rev
> Log:
> [Support] Switch to RAII helper for error-as-out-parameter idiom.
>
> As discussed on the llvm-commits thread for r264467.
>
> Modified:
> llvm/trunk/include/llvm/Support/Error.h
> llvm/trunk/lib/Object/MachOObjectFile.cpp
> 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=264479&r1=264478&r2=264479&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/Support/Error.h (original)
> +++ llvm/trunk/include/llvm/Support/Error.h Fri Mar 25 18:54:32 2016
> @@ -143,13 +143,6 @@ public:
> /// constructor, but should be preferred for readability where possible.
> static Error success() { return Error(); }
>
> - /// Create a 'pre-checked' success value suitable for use as an
> out-parameter.
> - static Error errorForOutParameter() {
> - Error Err;
> - (void)!!Err;
> - return Err;
> - }
> -
> // Errors are not copy-constructable.
> Error(const Error &Other) = delete;
>
> @@ -552,6 +545,36 @@ inline void consumeError(Error Err) {
> handleAllErrors(std::move(Err), [](const ErrorInfoBase &) {});
> }
>
> +/// Helper for Errors used as out-parameters.
> +///
> +/// This helper is for use with the Error-as-out-parameter idiom, where
> an error
> +/// is passed to a function or method by reference, rather than being
> returned.
> +/// In such cases it is helpful to set the checked bit on entry to the
> function
> +/// so that the error can be written to (unchecked Errors abort on
> assignment)
> +/// and clear the checked bit on exit so that clients cannot accidentally
> forget
> +/// to check the result. This helper performs these actions automatically
> using
> +/// RAII:
> +///
> +/// Result foo(Error &Err) {
> +/// ErrorAsOutParameter ErrAsOutParam(Err); // 'Checked' flag set
> +/// // <body of foo>
> +/// // <- 'Checked' flag auto-cleared when ErrAsOutParam is destructed.
> +/// }
> +class ErrorAsOutParameter {
> +public:
> + ErrorAsOutParameter(Error &Err) : Err(Err) {
> + // Raise the checked bit if Err is success.
> + (void)!!Err;
> + }
> + ~ErrorAsOutParameter() {
> + // Clear the checked bit.
> + if (!Err)
>
Wouldn't this raise the checked bit if there /was/ an error?
{ Error e; ErrorAsOutParameter(e); e = /* some actual error */; }
That should assert-fail, but I suspect it won't?
> + Err = Error::success();
> + }
> +private:
> + Error &Err;
> +};
> +
> /// Tagged union holding either a T or a Error.
> ///
> /// This class parallels ErrorOr, but replaces error_code with Error.
> Since
>
> Modified: llvm/trunk/lib/Object/MachOObjectFile.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/MachOObjectFile.cpp?rev=264479&r1=264478&r2=264479&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Object/MachOObjectFile.cpp (original)
> +++ llvm/trunk/lib/Object/MachOObjectFile.cpp Fri Mar 25 18:54:32 2016
> @@ -246,7 +246,7 @@ static Error parseSegmentLoadCommand(
> Expected<std::unique_ptr<MachOObjectFile>>
> MachOObjectFile::create(MemoryBufferRef Object, bool IsLittleEndian,
> bool Is64Bits) {
> - Error Err = Error::errorForOutParameter();
> + Error Err;
> std::unique_ptr<MachOObjectFile> Obj(
> new MachOObjectFile(std::move(Object), IsLittleEndian,
> Is64Bits, Err));
> @@ -262,7 +262,7 @@ MachOObjectFile::MachOObjectFile(MemoryB
> DataInCodeLoadCmd(nullptr), LinkOptHintsLoadCmd(nullptr),
> DyldInfoLoadCmd(nullptr), UuidLoadCmd(nullptr),
> HasPageZeroSegment(false) {
> -
> + ErrorAsOutParameter ErrAsOutParam(Err);
> if (is64Bit())
> parseHeader(this, Header64, Err);
> else
>
> Modified: llvm/trunk/unittests/Support/ErrorTest.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/ErrorTest.cpp?rev=264479&r1=264478&r2=264479&view=diff
>
> ==============================================================================
> --- llvm/trunk/unittests/Support/ErrorTest.cpp (original)
> +++ llvm/trunk/unittests/Support/ErrorTest.cpp Fri Mar 25 18:54:32 2016
> @@ -105,12 +105,32 @@ TEST(Error, UncheckedSuccess) {
> }
> #endif
>
> -// Test that errors to be used as out parameters are implicitly checked (
> -// and thus destruct quietly).
> -TEST(Error, ErrorAsOutParameter) {
> - Error E = Error::errorForOutParameter();
> +// ErrorAsOutParameter tester.
> +void errAsOutParamHelper(Error &Err) {
> + ErrorAsOutParameter ErrAsOutParam(Err);
> + // Verify that checked flag is raised - assignment should not crash.
> + Err = Error::success();
> + // Raise the checked bit manually - caller should still have to test the
> + // error.
> + (void)!!Err;
> }
>
> +// Test that ErrorAsOutParameter sets the checked flag on construction.
> +TEST(Error, ErrorAsOutParameterChecked) {
> + Error E;
> + errAsOutParamHelper(E);
> + (void)!!E;
> +}
> +
> +// Test that ErrorAsOutParameter clears the checked flag on destruction.
> +#ifndef NDEBUG
> +TEST(Error, ErrorAsOutParameterUnchecked) {
> + EXPECT_DEATH({ Error E; errAsOutParamHelper(E); },
> + "Program aborted due to an unhandled Error:")
> + << "ErrorAsOutParameter did not clear the checked flag on
> destruction.";
> +}
> +#endif
> +
> // Check that we abort on unhandled failure cases. (Force conversion to
> bool
> // to make sure that we don't accidentally treat checked errors as
> handled).
> // Test runs in debug mode only.
>
>
> _______________________________________________
> 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/20160325/730df170/attachment.html>
More information about the llvm-commits
mailing list