[PATCH] D55045: Add a version of std::function that includes a few optimizations.

Mon Dec 3 11:34:07 PST 2018

ldionne added a comment.

In D55045#1317092 <https://reviews.llvm.org/D55045#1317092>, @EricWF wrote:

> In D55045#1315603 <https://reviews.llvm.org/D55045#1315603>, @kristina wrote:
>
> > I don't see the issue with breaking ABI that's designated as unstable as long as there's some form of announcement to avoid any confusion. Fundamentally that's the entire point of having unstable ABI versions, some changes are just not really possible to make without ABI breakages, most consumers just use the stable ABI, while unstable ABI is not backwards compatible otherwise it would make a whole range of changes to `libc++` and/or Clang impossible.
>
>
> I don't see a problem breaking the unstable ABI either. But we don't normally standardize ABI breaking changes.

I fully agree we don't care about breaking an unstable ABI (since vendors won't ship it if they care about ABI stability). I think everyone is on the same page here.

> I think this means we should only ever have at most 2 versions of `std::function`. One for the stable ABI and  potentially an improved version for the unstable ABI.

This is where we disagree -- I don't think there's a universally better implementation of `std::function`. One implementation might be better in some circumstances and another implementation might be better in other circumstances, hence my claim that we should not go for a solution that assumes at most two implementations.

On a different note, I still don't understand why this `std::function` is better from a benchmark perspective. Some interpretation of those pastes would be appreciated, and we still don't seem to be sure what the output is (% or delta?). @EricWF should know, since he wrote GoogleBenchmark?

Sorry if I seem to be setting the bar a bit high, but I think we need this before we basically double the maintenance burden of `std::function`.

Repository:
  rCXX libc++

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

https://reviews.llvm.org/D55045