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

[Louis Dionne via Phabricator]

ldionne added a comment.

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

> In D55045#1317136 <https://reviews.llvm.org/D55045#1317136>, @ldionne wrote:
>
> > > 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.
>
>
> Our job is to provide a universal implementation of `std::function`, and hence pick one implementation which weights the different alternatives and chooses what we believe to be best in the majority of cases.


I agree, and that's why we need to select the best possible default implementation. I don't think that precludes giving users more choices. The current patch makes it harder to do that.

Similarly, if we were to stabilize ABI v2 with this new `std::function`, and if we eventually wanted to use yet another implementation for ABI v3, we'd now have three `#if` branches in each member function of `std::function`. I think it would be better to setup something better right now.

>> 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`.
> 
> Just above you suggested we get ready to add more than two implementations, but here you're setting a very high bar for adding one more?
>  I read this as a seeming contradiction.

In the current state of the patch, we're adding exactly one new implementation of `std::function`, and we're doing it in a way that it's "the one new better `std::function`". In that case, yes, I do want to understand what this new (better) `std::function` brings to the table. If we had a way of switching between multiple implementations more easily and this was just another implementation that users can pick if they wish to, I would almost certainly care a bit less.


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D55045