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

[Louis Dionne 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>


ldionne 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`.

@jwakely @CaseyCarter  Sorry to call you here directly, but since this is an ABI break for us (and we need to make it within ~2 weeks), I'd like to get an idea of what you folks think about this. To summarize the discussion, I'd like to confirm that the intent of the standard when specifying exposition-only members without `[[no_unique_address]]` is that the implementation doesn't use `[[no_unique_address]]` (which is not a transparent optimization based on what we learned in this thread). Have you folks run into this issue with your implementation of e.g. `expected`?

@huixie90 Assuming the answer's going to be "yup, `expected` shouldn't use `[[no_unique_address]]`", then we'll have to remove it from `expected` and cherry-pick to LLVM 17. We'll then need to audit our other usages of `[[no_unique_address]]` (we don't have that many, mostly in `<ranges>`) and make sure that we are allowed to use those -- along with tests. I expect that most of those are not as vexing as an ABI break in `expected` (which is a vocabulary type), so we might be fine with waiting for LLVM 18 to change those.

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