[libcxx-commits] [PATCH] D101206: [libc++] Remove UB in list, forward_list and __hash_table

Fri Oct 13 17:45:03 PDT 2023

Michael137 added a comment.

In D101206#4653905 <https://reviews.llvm.org/D101206#4653905>, @dblaikie wrote:

> In D101206#4653863 <https://reviews.llvm.org/D101206#4653863>, @ldionne wrote:
>
>> In D101206#4653478 <https://reviews.llvm.org/D101206#4653478>, @dblaikie wrote:
>>
>>> In D101206#4653421 <https://reviews.llvm.org/D101206#4653421>, @Michael137 wrote:
>>>
>>>> Proposed LLDB fix: https://github.com/llvm/llvm-project/pull/68574
>>>
>>> Personally - maybe this added complexity (combined with the existing slight increase in complexity of having #ifdef'd code) is enough to tip in favor of having the one portable implementation - despite the possible slight reduction in implementation debuggability, since it doesn't sound like anyone's here saying they really need that feature?
>>
>> I agree. However, after speaking with @Michael137 , it would actually make LLDB's life harder if I were to always use the C++03 variant because right now they only handle the union version of the code and the previous version of the code. @Michael137 tells me I can re-commit this now, so I'll do that.
>
> Oh, it looks like it would handle the 03 variant today - because it'd find the `__value_` member on the first lookup, then it only falls back to looking through the anonymous union if that fails? Or does that fail later on because the `__value_` member isn't of the expected type in the 03 case (since it's just the raw buffer).
>
> Pity to lose the ability to pretty print the 03 version :/

We can certainly add support for the C++03 format too. I don't think have any test coverage for C++03 but wouldn't be too difficult to add. Will have a look at that next week

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101206/new/

https://reviews.llvm.org/D101206