[libcxx-commits] [libcxx] [libc++] Fix UB in <expected> related to "has value" flag (#68552) (PR #68733)

[A. Jiang via libcxx-commits]

Jan =?utf-8?q?Kokemüller?= <jan.kokemueller at gmail.com>,
Jan =?utf-8?q?Kokemüller?= <jan.kokemueller at gmail.com>,
Jan =?utf-8?q?Kokemüller?= <jan.kokemueller at gmail.com>,
Jan =?utf-8?q?Kokemüller?= <jan.kokemueller at gmail.com>
Message-ID:
In-Reply-To: <llvm/llvm-project/pull/68733/libcxx at github.com>


frederick-vs-ja wrote:

> @frederick-vs-ja In other words, you're saying that you're pretty confident that the library wording (via exposition-only members) intends to prevent us from using `[[no_unique_address]]` and dodge the possibility of potentially-overlapping subobjects, even though that means we can't cram the `bool __has_value_` into the `_Tp` (which is a really nice optimization)?
> 
> I'm not disagreeing, just trying to understand. So if that's the case, there's no escaping from this one and we have to remove `[[no_unique_address]]`. However, I think this would probably mandate some clarification in the library wording because this optimization is pretty natural when implementing the class, and it seems pretty desirable -- probably more so than the ability for users to do shady stuff like playing with the lifetime of `val`/`unex`.

Yes. I guess it would be hard to use `construct_at`/`destroy_at` and direct assignment in the wording for `expected` if _`val`_ and _`unex`_ are allowed to be overlapping with _`has_val`_.

I'm not sure whether the rules in [[basic.life]](https://eel.is/c++draft/basic.life) allow us to replace such a potentially-overlapping _`val`_ or _`unex`_ (via `construct_at`) while keeping the containing `expected` object living. There're several CWG issues ([CWG2545](https://cplusplus.github.io/CWG/issues/2545.html), [CWG2675](https://cplusplus.github.io/CWG/issues/2675.html), [CWG2677](https://cplusplus.github.io/CWG/issues/2677.html)) about this.

Anyway, I think if we want to keep the current strategy using `[[no_unique_address]]`, then reconstruction of a union member may end the lifetime of `__has_value_` and thus perhaps `std::construct_at(__has_value_, true)` should be used instead of `__has_value_ = true`.

https://github.com/llvm/llvm-project/pull/68733