[libcxx-commits] [libcxx] [libc++][NFC] Remove `optional::__get` (PR #217218)
via libcxx-commits
libcxx-commits at lists.llvm.org
Tue Aug 18 23:19:41 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-libcxx
Author: A. Jiang (frederick-vs-ja)
<details>
<summary>Changes</summary>
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.
---
Full diff: https://github.com/llvm/llvm-project/pull/217218.diff
2 Files Affected:
- (modified) libcxx/include/__optional/optional.h (+23-29)
- (modified) libcxx/include/__optional/optional_ref.h (+5-7)
``````````diff
diff --git a/libcxx/include/__optional/optional.h b/libcxx/include/__optional/optional.h
index 7c2ab7574e8cb..df2f26e4be08d 100644
--- a/libcxx/include/__optional/optional.h
+++ b/libcxx/include/__optional/optional.h
@@ -154,11 +154,6 @@ 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_); }
-
template <class... _Args>
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __construct(_Args&&... __args) {
_LIBCPP_ASSERT_INTERNAL(!has_value(), "__construct called for engaged __optional_storage");
@@ -169,19 +164,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 +438,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 +462,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,13 +477,13 @@ 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();
}
}
@@ -497,74 +492,73 @@ class _LIBCPP_DECLSPEC_EMPTY_BASES optional
// [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();
+ 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->__get();
+ 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->__get());
+ 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->__get());
+ return std::move(this->__val_);
}
_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;
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 +696,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 +706,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>
``````````
</details>
https://github.com/llvm/llvm-project/pull/217218
More information about the libcxx-commits
mailing list