[llvm] r285426 - [Error] Unify +Asserts/-Asserts behavior for checked flags in Error/Expected<T>.
Lang Hames via llvm-commits
llvm-commits at lists.llvm.org
Fri Oct 28 11:24:16 PDT 2016
Author: lhames
Date: Fri Oct 28 13:24:15 2016
New Revision: 285426
URL: http://llvm.org/viewvc/llvm-project?rev=285426&view=rev
Log:
[Error] Unify +Asserts/-Asserts behavior for checked flags in Error/Expected<T>.
(1) Switches to raw pointer and bitmasking operations for Error payload.
(2) Always includes the 'unchecked' bitfield in Expected<T>, 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.
Modified:
llvm/trunk/include/llvm/Support/Error.h
Modified: llvm/trunk/include/llvm/Support/Error.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/Error.h?rev=285426&r1=285425&r2=285426&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/Error.h (original)
+++ llvm/trunk/include/llvm/Support/Error.h Fri Oct 28 13:24:15 2016
@@ -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<ErrorInfoBase*>(
+ reinterpret_cast<uintptr_t>(Payload) &
+ ~static_cast<uintptr_t>(0x1));
}
void setPtr(ErrorInfoBase *EI) {
#ifndef NDEBUG
- PayloadAndCheckedBit.setPointer(EI);
+ Payload = reinterpret_cast<ErrorInfoBase*>(
+ (reinterpret_cast<uintptr_t>(EI) &
+ ~static_cast<uintptr_t>(0x1)) |
+ (reinterpret_cast<uintptr_t>(Payload) & 0x1));
#else
Payload = EI;
#endif
@@ -255,16 +256,17 @@ private:
bool getChecked() const {
#ifndef NDEBUG
- return PayloadAndCheckedBit.getInt();
+ return (reinterpret_cast<uintptr_t>(Payload) & 0x1) == 0;
#else
return true;
#endif
}
void setChecked(bool V) {
-#ifndef NDEBUG
- PayloadAndCheckedBit.setInt(V);
-#endif
+ Payload = reinterpret_cast<ErrorInfoBase*>(
+ (reinterpret_cast<uintptr_t>(Payload) &
+ ~static_cast<uintptr_t>(0x1)) |
+ (V ? 0 : 1));
}
std::unique_ptr<ErrorInfoBase> takePayload() {
@@ -274,11 +276,7 @@ private:
return Tmp;
}
-#ifndef NDEBUG
- PointerIntPair<ErrorInfoBase *, 1> 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<T> 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<T> from Error success value.");
new (getErrorStorage()) Error(std::move(Err));
@@ -647,10 +651,15 @@ public:
Expected(OtherT &&Val,
typename std::enable_if<std::is_convertible<OtherT, T>::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<OtherT>(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<T> vaule.
Error takeError() {
#ifndef NDEBUG
- Checked = true;
+ Unchecked = false;
#endif
return HasError ? Error(std::move(*getErrorStorage())) : Error::success();
}
@@ -766,11 +775,8 @@ private:
template <class OtherT> void moveConstruct(Expected<OtherT> &&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<T> must be checked before access or destruction.\n";
if (HasError) {
dbgs() << "Unchecked Expected<T> contained error:\n";
@@ -832,9 +838,7 @@ private:
AlignedCharArrayUnion<error_type> ErrorStorage;
};
bool HasError : 1;
-#ifndef NDEBUG
- bool Checked : 1;
-#endif
+ bool Unchecked : 1;
};
/// This class wraps a std::error_code in a Error.
More information about the llvm-commits
mailing list