base::Optional: CHECK if attempting to dereference no set value.

base::Optional::value() already does this; change operator* and
operator-> to match, and ensure the behavior is tested.

Change-Id: Id686b86d0e68360dea3a92d01206aefde6782a16
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1531498
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#649250}

base/optional.h

@@ -568,32 +568,32 @@ class OPTIONAL_DECLSPEC_EMPTY_BASES Optional
   }
 
   constexpr const T* operator->() const {
-    DCHECK(storage_.is_populated_);
+    CHECK(storage_.is_populated_);
     return &storage_.value_;
   }
 
   constexpr T* operator->() {
-    DCHECK(storage_.is_populated_);
+    CHECK(storage_.is_populated_);
     return &storage_.value_;
   }
 
   constexpr const T& operator*() const & {
-    DCHECK(storage_.is_populated_);
+    CHECK(storage_.is_populated_);
     return storage_.value_;
   }
 
   constexpr T& operator*() & {
-    DCHECK(storage_.is_populated_);
+    CHECK(storage_.is_populated_);
     return storage_.value_;
   }
 
   constexpr const T&& operator*() const && {
-    DCHECK(storage_.is_populated_);
+    CHECK(storage_.is_populated_);
     return std::move(storage_.value_);
   }
 
   constexpr T&& operator*() && {
-    DCHECK(storage_.is_populated_);
+    CHECK(storage_.is_populated_);
     return std::move(storage_.value_);
   }
 

base/optional_unittest.cc

@@ -9,6 +9,7 @@
 #include <string>
 #include <vector>
 
+#include "base/test/gtest_util.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -2119,6 +2120,29 @@ TEST(OptionalTest, DontCallNewMemberFunction) {
   EXPECT_TRUE(a.has_value());
 }
 
+TEST(OptionalTest, DereferencingNoValueCrashes) {
+  class C {
+   public:
+    void Method() const {}
+  };
+
+  {
+    const Optional<C> const_optional;
+    EXPECT_CHECK_DEATH(const_optional.value());
+    EXPECT_CHECK_DEATH(const_optional->Method());
+    EXPECT_CHECK_DEATH(*const_optional);
+    EXPECT_CHECK_DEATH(*std::move(const_optional));
+  }
+
+  {
+    Optional<C> non_const_optional;
+    EXPECT_CHECK_DEATH(non_const_optional.value());
+    EXPECT_CHECK_DEATH(non_const_optional->Method());
+    EXPECT_CHECK_DEATH(*non_const_optional);
+    EXPECT_CHECK_DEATH(*std::move(non_const_optional));
+  }
+}
+
 TEST(OptionalTest, Noexcept) {
   // Trivial copy ctor, non-trivial move ctor, nothrow move assign.
   struct Test1 {