[libcxx-commits] [libcxx] [libc++][NFC] Remove `optional::__get` (PR #217218)

A. Jiang via libcxx-commits libcxx-commits at lists.llvm.org
Tue Aug 18 23:40:00 PDT 2026


https://github.com/frederick-vs-ja updated https://github.com/llvm/llvm-project/pull/217218

>From 79dbbf3ca2c639578d2b9ddeb3bb5799e24b5d24 Mon Sep 17 00:00:00 2001
From: "A. Jiang" <de34 at live.cn>
Date: Wed, 19 Aug 2026 14:15:40 +0800
Subject: [PATCH] [libc++][NFC] Remove `optional::__get`

Internal communicating functions `optional::__get` can be superseded by
either `operator*` or direct use of data member. Even though
`operator*()` is checked on hardening, checks are already performed
before internal calls to it, so compilers should be able to remove
duplicated checks.

It is intended to use `opt.operator*()` instead of `*opt` to avoid
undesired argument-dependent lookup.
---
 libcxx/include/__optional/optional.h     | 83 ++++++++++++------------
 libcxx/include/__optional/optional_ref.h | 12 ++--
 2 files changed, 45 insertions(+), 50 deletions(-)

diff --git a/libcxx/include/__optional/optional.h b/libcxx/include/__optional/optional.h
index 7c2ab7574e8cb..f462bb0449e85 100644
--- a/libcxx/include/__optional/optional.h
+++ b/libcxx/include/__optional/optional.h
@@ -154,10 +154,26 @@ struct __optional_storage_base : __optional_destruct_base<_Tp> {
 
   [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr bool has_value() const noexcept { return this->__engaged_; }
 
-  _LIBCPP_HIDE_FROM_ABI constexpr value_type& __get() & noexcept { return this->__val_; }
-  _LIBCPP_HIDE_FROM_ABI constexpr const value_type& __get() const& noexcept { return this->__val_; }
-  _LIBCPP_HIDE_FROM_ABI constexpr value_type&& __get() && noexcept { return std::move(this->__val_); }
-  _LIBCPP_HIDE_FROM_ABI constexpr const value_type&& __get() const&& noexcept { return std::move(this->__val_); }
+  // [optional.observe]
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr const _Tp& operator*() const& noexcept {
+    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(this->has_value(), "optional operator* called on a disengaged value");
+    return this->__val_;
+  }
+
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr _Tp& operator*() & noexcept {
+    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(this->has_value(), "optional operator* called on a disengaged value");
+    return this->__val_;
+  }
+
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr _Tp&& operator*() && noexcept {
+    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(this->has_value(), "optional operator* called on a disengaged value");
+    return std::move(this->__val_);
+  }
+
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr const _Tp&& operator*() const&& noexcept {
+    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(this->has_value(), "optional operator* called on a disengaged value");
+    return std::move(this->__val_);
+  }
 
   template <class... _Args>
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __construct(_Args&&... __args) {
@@ -169,19 +185,19 @@ struct __optional_storage_base : __optional_destruct_base<_Tp> {
   template <class _That>
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __construct_from(_That&& __opt) {
     if (__opt.has_value())
-      __construct(std::forward<_That>(__opt).__get());
+      __construct(std::forward<_That>(__opt).operator*());
   }
 
   template <class _That>
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __assign_from(_That&& __opt) {
     if (this->__engaged_ == __opt.has_value()) {
       if (this->__engaged_)
-        static_cast<_Tp&>(this->__val_) = std::forward<_That>(__opt).__get();
+        static_cast<_Tp&>(this->__val_) = std::forward<_That>(__opt).operator*();
     } else {
       if (this->__engaged_)
         this->reset();
       else
-        __construct(std::forward<_That>(__opt).__get());
+        __construct(std::forward<_That>(__opt).operator*());
     }
   }
 };
@@ -443,7 +459,7 @@ class _LIBCPP_DECLSPEC_EMPTY_BASES optional
                         int> = 0>
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 optional& operator=(_Up&& __v) {
     if (this->has_value())
-      this->__get() = std::forward<_Up>(__v);
+      this->__val_ = std::forward<_Up>(__v);
     else
       this->__construct(std::forward<_Up>(__v));
     return *this;
@@ -467,14 +483,14 @@ class _LIBCPP_DECLSPEC_EMPTY_BASES optional
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _Tp& emplace(_Args&&... __args) {
     reset();
     this->__construct(std::forward<_Args>(__args)...);
-    return this->__get();
+    return this->__val_;
   }
 
   template <class _Up, class... _Args, enable_if_t<is_constructible_v<_Tp, initializer_list<_Up>&, _Args...>, int> = 0>
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _Tp& emplace(initializer_list<_Up> __il, _Args&&... __args) {
     reset();
     this->__construct(__il, std::forward<_Args>(__args)...);
-    return this->__get();
+    return this->__val_;
   }
 
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void
@@ -482,89 +498,70 @@ class _LIBCPP_DECLSPEC_EMPTY_BASES optional
     using std::swap;
     if (this->has_value() == __opt.has_value()) {
       if (this->has_value())
-        swap(this->__get(), __opt.__get());
+        swap(this->__val_, __opt.__val_);
     } else {
       if (this->has_value()) {
-        __opt.__construct(std::move(this->__get()));
+        __opt.__construct(std::move(this->__val_));
         this->reset();
       } else {
-        this->__construct(std::move(__opt.__get()));
+        this->__construct(std::move(__opt.__val_));
         __opt.reset();
       }
     }
   }
 
   // [optional.observe]
-  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr const _Tp& operator*() const& noexcept {
-    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(this->has_value(), "optional operator* called on a disengaged value");
-    return this->__get();
-  }
-
-  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr _Tp& operator*() & noexcept {
-    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(this->has_value(), "optional operator* called on a disengaged value");
-    return this->__get();
-  }
-
-  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr _Tp&& operator*() && noexcept {
-    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(this->has_value(), "optional operator* called on a disengaged value");
-    return std::move(this->__get());
-  }
-
-  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr const _Tp&& operator*() const&& noexcept {
-    _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(this->has_value(), "optional operator* called on a disengaged value");
-    return std::move(this->__get());
-  }
   _LIBCPP_HIDE_FROM_ABI constexpr add_pointer_t<_Tp const> operator->() const noexcept {
     _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(this->has_value(), "optional operator-> called on a disengaged value");
-    return std::addressof(this->__get());
+    return std::addressof(this->__val_);
   }
 
   _LIBCPP_HIDE_FROM_ABI constexpr add_pointer_t<_Tp> operator->() noexcept {
     _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(this->has_value(), "optional operator-> called on a disengaged value");
-    return std::addressof(this->__get());
+    return std::addressof(this->__val_);
   }
 
   [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr _Tp const& value() const& {
     if (!this->has_value())
       std::__throw_bad_optional_access();
-    return this->__get();
+    return this->__val_;
   }
 
   [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr _Tp& value() & {
     if (!this->has_value())
       std::__throw_bad_optional_access();
-    return this->__get();
+    return this->__val_;
   }
 
   [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr _Tp&& value() && {
     if (!this->has_value())
       std::__throw_bad_optional_access();
-    return std::move(this->__get());
+    return std::move(this->__val_);
   }
 
   [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr _Tp const&& value() const&& {
     if (!this->has_value())
       std::__throw_bad_optional_access();
-    return std::move(this->__get());
+    return std::move(this->__val_);
   }
 
   _LIBCPP_HIDE_FROM_ABI constexpr explicit operator bool() const noexcept { return has_value(); }
 
-  using __base::__get;
   using __base::has_value;
+  using __base::operator*;
 
   template <class _Up = remove_cv_t<_Tp>>
   [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr _Tp value_or(_Up&& __v) const& {
     static_assert(is_copy_constructible_v<_Tp>, "optional<T>::value_or: T must be copy constructible");
     static_assert(is_convertible_v<_Up, _Tp>, "optional<T>::value_or: U must be convertible to T");
-    return this->has_value() ? this->__get() : static_cast<_Tp>(std::forward<_Up>(__v));
+    return this->has_value() ? this->__val_ : static_cast<_Tp>(std::forward<_Up>(__v));
   }
 
   template <class _Up = remove_cv_t<_Tp>>
   [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr _Tp value_or(_Up&& __v) && {
     static_assert(is_move_constructible_v<_Tp>, "optional<T>::value_or: T must be move constructible");
     static_assert(is_convertible_v<_Up, _Tp>, "optional<T>::value_or: U must be convertible to T");
-    return this->has_value() ? std::move(this->__get()) : static_cast<_Tp>(std::forward<_Up>(__v));
+    return this->has_value() ? std::move(this->__val_) : static_cast<_Tp>(std::forward<_Up>(__v));
   }
 
 #  if _LIBCPP_STD_VER >= 23
@@ -702,7 +699,7 @@ class _LIBCPP_DECLSPEC_EMPTY_BASES optional
 
   // [optional.iterators], iterator support
   [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr iterator begin() noexcept {
-    auto* __ptr = std::addressof(this->__get());
+    auto* __ptr = std::addressof(this->__val_);
 
 #    ifdef _LIBCPP_ABI_BOUNDED_ITERATORS_IN_OPTIONAL
     return std::__make_bounded_iter(__ptr, __ptr, __ptr + (this->has_value() ? 1 : 0));
@@ -712,7 +709,7 @@ class _LIBCPP_DECLSPEC_EMPTY_BASES optional
   }
 
   [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr const_iterator begin() const noexcept {
-    auto* __ptr = std::addressof(this->__get());
+    auto* __ptr = std::addressof(this->__val_);
 
 #    ifdef _LIBCPP_ABI_BOUNDED_ITERATORS_IN_OPTIONAL
     return std::__make_bounded_iter(__ptr, __ptr, __ptr + (this->has_value() ? 1 : 0));
diff --git a/libcxx/include/__optional/optional_ref.h b/libcxx/include/__optional/optional_ref.h
index 2fa8c474212e7..6d3b328010946 100644
--- a/libcxx/include/__optional/optional_ref.h
+++ b/libcxx/include/__optional/optional_ref.h
@@ -107,7 +107,7 @@ class optional<_Tp&> : public __optional_ref_iterator_base<_Tp&> {
   template <class _That>
   _LIBCPP_HIDE_FROM_ABI constexpr void __construct_from(_That&& __opt) {
     if (__opt.has_value())
-      __construct(std::forward<_That>(__opt).__get());
+      __construct(std::forward<_That>(__opt).operator*());
   }
 
   static constexpr bool __has_iterator_ = requires { typename __optional_ref_iterator_base<_Tp&>::iterator; };
@@ -190,8 +190,6 @@ class optional<_Tp&> : public __optional_ref_iterator_base<_Tp&> {
 
   constexpr ~optional() = default;
 
-  _LIBCPP_HIDE_FROM_ABI constexpr value_type& __get() const noexcept { return *__value_; }
-
   _LIBCPP_HIDE_FROM_ABI constexpr optional& operator=(nullopt_t) noexcept {
     reset();
     return *this;
@@ -204,7 +202,7 @@ class optional<_Tp&> : public __optional_ref_iterator_base<_Tp&> {
   constexpr _Tp& emplace(_Up&& __u) noexcept(is_nothrow_constructible_v<_Tp&, _Up>) {
     this->__construct(std::forward<_Up>(__u));
 
-    return this->__get();
+    return *__value_;
   }
 
   constexpr void swap(optional& __rhs) noexcept { std::swap(this->__value_, __rhs.__value_); }
@@ -217,7 +215,7 @@ class optional<_Tp&> : public __optional_ref_iterator_base<_Tp&> {
 
   [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr _Tp& operator*() const noexcept {
     _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS(this->has_value(), "optional operator* called on a disengaged value");
-    return this->__get();
+    return *__value_;
   }
 
   constexpr explicit operator bool() const noexcept { return has_value(); }
@@ -227,7 +225,7 @@ class optional<_Tp&> : public __optional_ref_iterator_base<_Tp&> {
   [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr _Tp& value() const {
     if (!this->has_value())
       std::__throw_bad_optional_access();
-    return this->__get();
+    return *__value_;
   }
 
   template <class _Up = remove_cv_t<_Tp>>
@@ -236,7 +234,7 @@ class optional<_Tp&> : public __optional_ref_iterator_base<_Tp&> {
     using _XTp = remove_cv_t<_Tp>;
     static_assert(is_constructible_v<_XTp, _Tp&>, "optional<T&>::value_or: remove_cv_t<T> must be constructible");
     static_assert(is_convertible_v<_Up, _XTp>, "optional<T&>::value_or: U must be convertible to remove_cv_t<T>");
-    return this->has_value() ? this->__get() : static_cast<_XTp>(std::forward<_Up>(__v));
+    return this->has_value() ? *__value_ : static_cast<_XTp>(std::forward<_Up>(__v));
   }
 
   template <class _Func>



More information about the libcxx-commits mailing list