[libcxx-commits] [libcxx] [libc++] Make sure LWG2070 is implemented as a DR (PR #65998)

[Louis Dionne via libcxx-commits]

================
@@ -321,7 +323,9 @@ struct __shared_ptr_emplace
             allocator_traits<_TpAlloc>::destroy(__tmp, __get_elem());
         }
 #else
-        __get_elem()->~_Tp();
+        using _TpAlloc = typename __allocator_traits_rebind<_Alloc, _Tp>::type;
----------------
ldionne wrote:

You make a great point about this `#ifdef` being in a virtual function, which could lead to ODR violations if two TUs both contain a definition of the function but are compiled with different standard modes. We can't drop the `#if` entirely because `if constexpr` doesn't work before C++17, and this code needs to work all the way back to C++03.

That being said, any ODR violation would necessarily be benign, so I don't think this really has been a problem. Indeed, `__for_overwrite_tag` is only passed when `allocate_shared_for_overwrite` is used, and that function is only provided in C++20 mode. Hence, in a TU compiled with C++17 and earlier, this function would always be as follows (if we used the same definition in all standard modes):

```
if constexpr (false) {
    __get_elem()->~_Tp();
} else {
    using _TpAlloc = typename __allocator_traits_rebind<_Alloc, _Tp>::type;
    _TpAlloc __tmp(*__get_alloc());
    allocator_traits<_TpAlloc>::destroy(__tmp, __get_elem());
}
```

So whether the linker decides to select that function or the function defined as

```
using _TpAlloc = typename __allocator_traits_rebind<_Alloc, _Tp>::type;
_TpAlloc __tmp(*__get_alloc());
allocator_traits<_TpAlloc>::destroy(__tmp, __get_elem());
```

is irrelevant. That being said, I think it's easy to fix this problem so I'll do it in this patch. And this certainly was a good catch!

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