[libcxx-commits] [PATCH] D149292: [libc++] Fix ODR violation with placeholders
Louis Dionne via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Wed Apr 26 13:43:46 PDT 2023
ldionne created this revision.
Herald added a project: All.
ldionne requested review of this revision.
Herald added a project: libc++.
Herald added a subscriber: libcxx-commits.
Herald added a reviewer: libc++.
In D145589 <https://reviews.llvm.org/D145589>, we made the std::bind placeholders inline constexpr to
satisfy C++17. It turns out that this causes ODR violations since the
shared library provides strong definitions for those placeholders, and
the linker on Windows actually complains about this.
Fortunately, C++17 only encourages implementations to use `inline constexpr`,
it doesn't force them. So instead, we unconditionally define the placeholders
as `extern const`, which avoids the ODR violation and is indistinguishable
from `inline constexpr` for most purposes, since the placeholders are
empty types anyway.
Note that we could also go back to the pre-D145589 state of defining them
as non-inline constexpr variables in C++17, however that is definitely
non-conforming since that means the placeholders have different addresses
in different TUs. This is all a bit pedantic, but all in all I feel that
`extern const` provides the best bang for our buck, and I can't really
find any downsides to that solution.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D149292
Files:
libcxx/include/__functional/bind.h
libcxx/test/std/utilities/function.objects/bind/func.bind/func.bind.place/placeholders.pass.cpp
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D149292.517306.patch
Type: text/x-patch
Size: 6827 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20230426/9b14832f/attachment.bin>
More information about the libcxx-commits
mailing list