From 60699bf49a6c9bb6ec1bb3601ab329657e8bcb7d Mon Sep 17 00:00:00 2001 From: Lang Hames Date: Fri, 28 Oct 2016 18:24:15 +0000 Subject: [PATCH] [Error] Unify +Asserts/-Asserts behavior for checked flags in Error/Expected. (1) Switches to raw pointer and bitmasking operations for Error payload. (2) Always includes the 'unchecked' bitfield in Expected, even in -Asserts. (3) Always propagates checked bit status in move-ops for both classes, even in -Asserts. This should allow debug programs to link against release libraries without encountering spurious 'unchecked error' terminations. Error checks still aren't verified in release mode so this doesn't introduce any new control flow, but it does require new bit-masking ops in release mode to preserve the flag values during move ops. I expect the overhead to be minimal, but if we discover any corner cases where it matters we could fix this by making flag propagation conditional on a new build option. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@285426 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/Support/Error.h | 70 +++++++++++++++++++++++--------------------- 1 file changed, 37 insertions(+), 33 deletions(-) diff --git a/include/llvm/Support/Error.h b/include/llvm/Support/Error.h index d228a0fbdbc..0f9b3260385 100644 --- a/include/llvm/Support/Error.h +++ b/include/llvm/Support/Error.h @@ -152,7 +152,7 @@ class LLVM_NODISCARD Error { public: /// Create a success value. Prefer using 'Error::success()' for readability /// where possible. - Error() { + Error() : Payload(nullptr) { setPtr(nullptr); setChecked(false); } @@ -167,7 +167,7 @@ public: /// Move-construct an error value. The newly constructed error is considered /// unchecked, even if the source error had been checked. The original error /// becomes a checked Success value, regardless of its original state. - Error(Error &&Other) { + Error(Error &&Other) : Payload(nullptr) { setChecked(true); *this = std::move(Other); } @@ -238,16 +238,17 @@ private: } ErrorInfoBase *getPtr() const { -#ifndef NDEBUG - return PayloadAndCheckedBit.getPointer(); -#else - return Payload; -#endif + return reinterpret_cast( + reinterpret_cast(Payload) & + ~static_cast(0x1)); } void setPtr(ErrorInfoBase *EI) { #ifndef NDEBUG - PayloadAndCheckedBit.setPointer(EI); + Payload = reinterpret_cast( + (reinterpret_cast(EI) & + ~static_cast(0x1)) | + (reinterpret_cast(Payload) & 0x1)); #else Payload = EI; #endif @@ -255,16 +256,17 @@ private: bool getChecked() const { #ifndef NDEBUG - return PayloadAndCheckedBit.getInt(); + return (reinterpret_cast(Payload) & 0x1) == 0; #else return true; #endif } void setChecked(bool V) { -#ifndef NDEBUG - PayloadAndCheckedBit.setInt(V); -#endif + Payload = reinterpret_cast( + (reinterpret_cast(Payload) & + ~static_cast(0x1)) | + (V ? 0 : 1)); } std::unique_ptr takePayload() { @@ -274,11 +276,7 @@ private: return Tmp; } -#ifndef NDEBUG - PointerIntPair PayloadAndCheckedBit; -#else ErrorInfoBase *Payload; -#endif }; /// Make a Error instance representing failure using the given error info @@ -631,11 +629,17 @@ private: public: /// Create an Expected error value from the given Error. Expected(Error Err) - : HasError(true) + : HasError(true), #ifndef NDEBUG - , - Checked(false) + // Expected is unchecked upon construction in Debug builds. + Unchecked(true) +#else + // Expected's unchecked flag is set to false in Release builds. This + // allows Expected values constructed in a Release build library to be + // consumed by a Debug build application. + Unchecked(false) #endif + { assert(Err && "Cannot create Expected from Error success value."); new (getErrorStorage()) Error(std::move(Err)); @@ -647,10 +651,15 @@ public: Expected(OtherT &&Val, typename std::enable_if::value>::type * = nullptr) - : HasError(false) + : HasError(false), #ifndef NDEBUG - , - Checked(false) + // Expected is unchecked upon construction in Debug builds. + Unchecked(true) +#else + // Expected's 'unchecked' flag is set to false in Release builds. This + // allows Expected values constructed in a Release build library to be + // consumed by a Debug build application. + Unchecked(false) #endif { new (getStorage()) storage_type(std::forward(Val)); @@ -696,7 +705,7 @@ public: /// \brief Return false if there is an error. explicit operator bool() { #ifndef NDEBUG - Checked = !HasError; + Unchecked = HasError; #endif return !HasError; } @@ -724,7 +733,7 @@ public: /// be made on the Expected vaule. Error takeError() { #ifndef NDEBUG - Checked = true; + Unchecked = false; #endif return HasError ? Error(std::move(*getErrorStorage())) : Error::success(); } @@ -766,11 +775,8 @@ private: template void moveConstruct(Expected &&Other) { HasError = Other.HasError; - -#ifndef NDEBUG - Checked = false; - Other.Checked = true; -#endif + Unchecked = true; + Other.Unchecked = false; if (!HasError) new (getStorage()) storage_type(std::move(*Other.getStorage())); @@ -813,7 +819,7 @@ private: void assertIsChecked() { #ifndef NDEBUG - if (!Checked) { + if (Unchecked) { dbgs() << "Expected must be checked before access or destruction.\n"; if (HasError) { dbgs() << "Unchecked Expected contained error:\n"; @@ -832,9 +838,7 @@ private: AlignedCharArrayUnion ErrorStorage; }; bool HasError : 1; -#ifndef NDEBUG - bool Checked : 1; -#endif + bool Unchecked : 1; }; /// This class wraps a std::error_code in a Error. -- 2.11.0