[libcxx-commits] [libcxx] [libc++] Cooperation between `std::optional` and other standard types (PR #93672)
Mital Ashok via libcxx-commits
libcxx-commits at lists.llvm.org
Thu May 30 05:14:30 PDT 2024
MitalAshok wrote:
I've tried to make it so that no operations (except `__optional::__cooperate` operations like `__is_engaged`) are done on an object in an invalid state (including calling the destructor), so no library functions need to special case for "is disengaged" except `optional::has_value`/`optional::__construct`. There is also a chance that standard library classes become constexpr in the future, so using padding bits isn't really an option.
In `std::optional<std::reference_wrapper<T>>`'s case, when a disengaged optional is constructed, it might have been zeroed over already, meaning this pull request doesn't change the behaviour if it is used unchecked. I don't think `std::reference_wrapper` will allow null references in the future making this pretty safe, but if it ever did, it can be changed back in the latest unstable ABI. E.g., when `_LIBCPP_ABI_VERSION=2` is stable and `_LIBCPP_ABI_VERSION=3` is released, it can look like:
```c++
template <class _Tp>
struct __optional::__cooperate<reference_wrapper<_Tp>> {
_LIBCPP_HIDE_FROM_ABI static constexpr bool __do_cooperate() {
#if _LIBCPP_ABI_VERSION < 3
return true;
#else
return false;
#endif
}
```
And in terms of performance, for `optional<reference_wrapper<T>>` specifically, it should become equivalent to a `T*` pointer. `has_value` goes from "read a bool" to "read the value then compare it to `nullptr`", but there are savings in that the type is smaller so less registers need to be allocated and so on. The read value will likely be used again if you checked `has_value` too (in the common pattern `if (auto Opt = get_optional()) { auto Val = *Opt; ... }`). I managed to get it working on Compiler Explorer so you can see the code gen live: https://godbolt.org/z/nzGqTbcW6 . It's also equivalent to how `optional<T&>` is implemented currently in libc++ https://github.com/llvm/llvm-project/blob/8b600a37325bd68c370b00838c9f0a0fda1af6ce/libcxx/include/optional#L386-L393
For `optional<function>`, I was thinking that the "disengaged" state be that the pointer is to some constant known value, so `has_value` would read the value and compare it to some other value, which should be more performant for the same reasons as above.
---
Also the `__builtin_is_within_lifetime` stuff is mostly to support use of `is_within_lifetime` itself. E.g., this code:
```c++
extern int i;
consteval bool f() {
std::optional<std::reference_wrapper<int>> opt(i);
std::reference_wrapper<int>& r = *opt;
assert(std::is_within_lifetime(r));
opt.reset();
assert(!std::is_within_lifetime(r));
return true;
}
static_assert(f());
```
When `is_within_lifetime` is not available, it is pretty difficult to observe the library UB of keeping around the reference to a now-disengaged optional
https://github.com/llvm/llvm-project/pull/93672
More information about the libcxx-commits
mailing list