[all-commits] [llvm/llvm-project] 77ac36: [libc++] Fix ODR violation with placeholders

Louis Dionne via All-commits all-commits at lists.llvm.org
Thu Apr 27 07:57:35 PDT 2023


  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 77ac36547a2d15dc465d723179ce7047a59583ce
      https://github.com/llvm/llvm-project/commit/77ac36547a2d15dc465d723179ce7047a59583ce
  Author: Louis Dionne <ldionne.2 at gmail.com>
  Date:   2023-04-27 (Thu, 27 Apr 2023)

  Changed paths:
    M libcxx/include/__functional/bind.h
    M libcxx/test/std/utilities/function.objects/bind/func.bind/func.bind.place/placeholders.pass.cpp

  Log Message:
  -----------
  [libc++] Fix ODR violation with placeholders

In 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.

Differential Revision: https://reviews.llvm.org/D149292




More information about the All-commits mailing list