From 292e8533b882503ce5a74b154f3a9e7ba744dbd8 Mon Sep 17 00:00:00 2001
From: Rafael Espindola <rafael.espindola@gmail.com>
Date: Tue, 5 Nov 2013 23:41:57 +0000
Subject: [PATCH] Remove another unused, and IMHO, not very desirable feature
 of ErrorOr.

One of the uses of the IsValid flag is to support default constructing
a ErrorOr that is not a Error or a Value. There is not much value in
doing that IMHO. If ErrorOr was to have a default constructor, it
should be implemented by default constructing the value, but even that
looks unnecessary.

The other use is to avoid calling destructors on moved objects. This
looks wrong. If the data being moved has non trivial treatment of
moves (an std::vector for example), it is its destructor that should
handle it, not ~ErrorOr.

With this change ErrorOr becomes a fairly simple wrapper and should
always be better than using an error_code + value in an API.

llvm-svn: 194109
---
 include/llvm/Support/ErrorOr.h | 36 +++++++---------------------------
 1 file changed, 7 insertions(+), 29 deletions(-)

diff --git a/include/llvm/Support/ErrorOr.h b/include/llvm/Support/ErrorOr.h
index 0fdafa09d58..d5b11cbe522 100644
--- a/include/llvm/Support/ErrorOr.h
+++ b/include/llvm/Support/ErrorOr.h
@@ -108,30 +108,28 @@ private:
   typedef typename remove_reference<T>::type *pointer;
 
 public:
-  ErrorOr() : IsValid(false) {}
-
   template <class E>
   ErrorOr(E ErrorCode, typename enable_if_c<is_error_code_enum<E>::value ||
                                             is_error_condition_enum<E>::value,
                                             void *>::type = 0)
-      : HasError(true), IsValid(true) {
+      : HasError(true) {
     new (getError()) error_code(make_error_code(ErrorCode));
   }
 
-  ErrorOr(llvm::error_code EC) : HasError(true), IsValid(true) {
+  ErrorOr(llvm::error_code EC) : HasError(true) {
     new (getError()) error_code(EC);
   }
 
-  ErrorOr(T Val) : HasError(false), IsValid(true) {
+  ErrorOr(T Val) : HasError(false) {
     new (get()) storage_type(moveIfMoveConstructible<storage_type>(Val));
   }
 
-  ErrorOr(const ErrorOr &Other) : IsValid(false) {
+  ErrorOr(const ErrorOr &Other) {
     copyConstruct(Other);
   }
 
   template <class OtherT>
-  ErrorOr(const ErrorOr<OtherT> &Other) : IsValid(false) {
+  ErrorOr(const ErrorOr<OtherT> &Other) {
     copyConstruct(Other);
   }
 
@@ -147,12 +145,12 @@ public:
   }
 
 #if LLVM_HAS_RVALUE_REFERENCES
-  ErrorOr(ErrorOr &&Other) : IsValid(false) {
+  ErrorOr(ErrorOr &&Other) {
     moveConstruct(std::move(Other));
   }
 
   template <class OtherT>
-  ErrorOr(ErrorOr<OtherT> &&Other) : IsValid(false) {
+  ErrorOr(ErrorOr<OtherT> &&Other) {
     moveConstruct(std::move(Other));
   }
 
@@ -169,8 +167,6 @@ public:
 #endif
 
   ~ErrorOr() {
-    if (!IsValid)
-      return;
     if (!HasError)
       get()->~storage_type();
   }
@@ -180,12 +176,10 @@ public:
 
   /// \brief Return false if there is an error.
   operator unspecified_bool_type() const {
-    assert(IsValid && "Can't do anything on a default constructed ErrorOr!");
     return HasError ? 0 : unspecified_bool_true;
   }
 
   operator llvm::error_code() const {
-    assert(IsValid && "Can't do anything on a default constructed ErrorOr!");
     return HasError ? *getError() : llvm::error_code::success();
   }
 
@@ -200,10 +194,6 @@ public:
 private:
   template <class OtherT>
   void copyConstruct(const ErrorOr<OtherT> &Other) {
-    // Construct an invalid ErrorOr if other is invalid.
-    if (!Other.IsValid)
-      return;
-    IsValid = true;
     if (!Other.HasError) {
       // Get the other value.
       HasError = false;
@@ -237,22 +227,14 @@ private:
 #if LLVM_HAS_RVALUE_REFERENCES
   template <class OtherT>
   void moveConstruct(ErrorOr<OtherT> &&Other) {
-    // Construct an invalid ErrorOr if other is invalid.
-    if (!Other.IsValid)
-      return;
-    IsValid = true;
     if (!Other.HasError) {
       // Get the other value.
       HasError = false;
       new (get()) storage_type(std::move(*Other.get()));
-      // Tell other not to do any destruction.
-      Other.IsValid = false;
     } else {
       // Get other's error.
       HasError = true;
       new (getError()) error_code(Other);
-      // Tell other not to do any destruction.
-      Other.IsValid = false;
     }
   }
 
@@ -275,19 +257,16 @@ private:
   }
 
   storage_type *get() {
-    assert(IsValid && "Can't do anything on a default constructed ErrorOr!");
     assert(!HasError && "Cannot get value when an error exists!");
     return reinterpret_cast<storage_type*>(TStorage.buffer);
   }
 
   const storage_type *get() const {
-    assert(IsValid && "Can't do anything on a default constructed ErrorOr!");
     assert(!HasError && "Cannot get value when an error exists!");
     return reinterpret_cast<const storage_type*>(TStorage.buffer);
   }
 
   error_code *getError() {
-    assert(IsValid && "Can't do anything on a default constructed ErrorOr!");
     assert(HasError && "Cannot get error when a value exists!");
     return reinterpret_cast<error_code*>(ErrorStorage.buffer);
   }
@@ -302,7 +281,6 @@ private:
     AlignedCharArrayUnion<error_code> ErrorStorage;
   };
   bool HasError : 1;
-  bool IsValid : 1;
 };
 
 template<class T, class E>