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

Kristina Brooks via Phabricator reviews at reviews.llvm.org
Thu Nov 29 06:16:49 PST 2018


kristina added subscribers: rjmccall, llvm-commits.
kristina added a comment.

> Subject: Re: [PATCH] D55045 <https://reviews.llvm.org/D55045>: Add a version of std::function that includes a few optimizations.
>  From: Jordan Soyke <jsoyke at google.com>
> 
> Do we need an additional config option for this?
> 
> On Thu, Nov 29, 2018 at 07:56 Kristina Brooks via Phabricator <reviews at reviews.llvm.org> wrote:
> 
>> kristina added reviewers: ldionne, EricWF, kristina, phosek.
>>  kristina added a comment.
>> 
>> Added reviewers. This will be a break in ABIv2 and Unstable ABI (since
>>  they're the same at the moment), only Fuchsia uses it at the moment, as far
>>  as I'm aware.

Not as far as I'm aware, since it's still considered unstable, but I figured it would be a good idea to make sure everyone is aware before builds suddenly start breaking for anyone who has `LIBCXX_ABI_VERSION=2`. The ABI stuff is confusing because currently unstable ABI and setting libc++ ABI to anything above 1 will cause the same effect, but if the ABI number is not set, nothing clobbers the `std::__1` namespace with unstable on (that causes havoc as it gets around versioned namespaces). Fuchsia explicitly uses ABI version 2, however some downstream projects build libc++ with vendor namespaces which likely make use of some "unstable" features (Facebook may be one example, I'm not sure).

I'm not actually sure about a lot of this since I'm still waiting to hear back from @rjmccall regarding Itanium ABI stdlib prefixes as well since that may pave the road for short mangling for common stdlib types, which as far as I understand requires explicitly reserving `std::__1` and `std::__2` for `libc++` and `std::__7` and `std::__8` for `libstdcxx` as they also use that scheme (short mangling was broken since the introduction of inline namespaces since currently only root namespace `std::` allows short mangling for certain types; there's a separate discussion regarding that here: https://github.com/itanium-cxx-abi/cxx-abi/pull/69). Incidentally that would also cause a complete breakage of unstable libc++ ABI.

Anyway, though I do like this optimization, it's probably a good idea to CC more people in on this, I'm way under-qualified to properly comment on this.


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D55045





More information about the libcxx-commits mailing list