[libcxx-commits] [PATCH] D145587: [libc++] Remove symbols for a std::allocator_arg & friends from the dylib

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a comment.

For reviewers -- note that we could technically keep defining those symbols in the dylib. I could define a `.cpp` file with something like

  struct defer_lock_t { explicit defer_lock_t() = default; };
  struct try_to_lock_t { explicit try_to_lock_t() = default; };
  struct adopt_lock_t { explicit adopt_lock_t() = default; };
  
  const defer_lock_t  defer_lock{};
  const try_to_lock_t try_to_lock{};
  const adopt_lock_t  adopt_lock{};
  
  // etc... for others

That way, we would prevent the list of exported symbols from changing and we would still support the (assumed to be) rare previously-built programs that rely on those symbols. However, that would also put all the other conforming programs in the situation of a (probably benign) ODR violation, since there would now be both a symbol provided in the dylib and an `inline constexpr` symbol defined via the headers. I know this is flagged by at least one implementation -- the linker on Windows will actually error out if we do that, detecting that there's a duplicate symbol. This is actually how I realized this issue and got to this patch series in the first place.

So I think we have to pick our poison. We either break things in a presumably small way today and then we get to live the rest of our lives in a clean world, or we decide to live with those symbols in the dylib forever and with (presumably benign) ODR violations in valid programs forever. Given that I expect the scope of breakage to be small, I am biased towards the first option and that is what this patch realizes, but I am open to discussion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145587