Page MenuHomePhabricator

[libc++][ABI BREAK] Remove the C++03 emulation for std::nullptr_t
Needs ReviewPublic

Authored by ldionne on Sep 8 2021, 12:09 PM.

Details

Reviewers
danalbert
ldionne
Group Reviewers
Restricted Project
Summary

We only support Clangs that implement nullptr as an extension in C++03 mode, and we don't support GCC in C++03 mode. Hence, we can technically drop our emulation for std::nullptr_t in C++03 mode. Doing that is technically an ABI break since it changes the mangling for std::nullptr_t. However:

(1) The only affected users are those compiling in C++03 mode that have std::nullptr_t as part of their ABI, which should be reasonably rare.

(2) Those users already have a lingering problem in that their code will be incompatible in C++03 and C++11 modes because of that very ABI break. Hence, the only users that could really be inconvenienced about this change is those that planned on compiling in C++03 mode forever - for other users, we're just breaking them now instead of letting them break themselves later on when they try to upgrade to C++11.

(3) The ABI break will cause a linker error since the mangling changed, and will not result in an obscure runtime error.

Concretely, the motivation for making this change is to make our own ABI consistent in C++03 and C++11 modes and to remove complexity around the definition of nullptr.

Furthermore, we could investigate making nullptr a keyword in C++03 mode as a Clang extension -- I don't think that would break anyone, since libc++ already defines nullptr as a macro to something else. Only users that do not use libc++ and compile in C++03 mode could potentially be broken by that.

Diff Detail

Event Timeline

ldionne created this revision.Sep 8 2021, 12:09 PM
ldionne requested review of this revision.Sep 8 2021, 12:09 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 8 2021, 12:09 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
ldionne edited the summary of this revision. (Show Details)Sep 8 2021, 12:10 PM

I'm putting this change up both for discussion and also to give people the chance to run test builds on internal code bases using this change. I think this change hinges quite a bit on the assumption that people almost never rely on std::nullptr_t in their ABI in C++03 mode. If that assumption turns out to be false, we may need to revisit the feasibility of this change.

Pinging some vendors to see what they think about this. @danalbert @phosek @dim @emaste

Perhaps @cjdb could run a build at Google? I'll do it at Apple.

FWLIW, I'm generally in favor of this. Although I think it's ironic that we already had an ABI flag for this, such that we'd have gotten this change for free if we ever managed to reach ABIv2. Feels like this PR is the most public admission yet that we'll never actually be able to reach std::__2.

libcxx/include/cstddef
46

Surely this should just unconditionally say typedef decltype(nullptr) nullptr_t; — why would we ever not want std::nullptr_t to exist?
(Vice versa, personally I don't understand why we define a type named ::nullptr_t; but I assume there's some good reason not to remove it now.)

ldionne marked an inline comment as done.Sep 8 2021, 12:47 PM

FWLIW, I'm generally in favor of this. Although I think it's ironic that we already had an ABI flag for this, such that we'd have gotten this change for free if we ever managed to reach ABIv2. Feels like this PR is the most public admission yet that we'll never actually be able to reach std::__2.

I don't think that's accurate. IMO, there are things we need to guard behind an ABI flag because they would actually break people and cause problems if we did not. Then there are things (like this nullptr_t emulation) that wouldn't break anyone anyways if we turned it on, so we might as well do it in the ABI v1 (proof is needed regarding whether I'm right about that, and I'm running builds to check that out right now).

IMO, the idea of an ABI v2 is a bit funky, I think we need to define it better. I think something more useful would be to formalize how different platforms get to select different ABI flags, since "breaking" the ABI is easiest done when one introduces a new platform. That doesn't happen super often, but it's better than nothing. Anyway, this is a discussion for a different forum.

libcxx/include/cstddef
46

Sure, I can drop _LIBCPP_USING_IF_EXISTS here since it's always defined (by us).

Regarding ::nullptr_t, I was surprised to read that <stddef.h> is supposed to provide such a type 🤷🏼‍♂️.

ldionne updated this revision to Diff 371421.Sep 8 2021, 12:51 PM
ldionne marked an inline comment as done.

Try to fix build on GCC and address Arthur's comment about using_if_exists

ldionne updated this revision to Diff 371575.Sep 9 2021, 6:19 AM

Fix the modules build.

danalbert accepted this revision.Sep 10 2021, 2:42 PM

I think this is probably fine from the NDK perspective. Even before Clang defaulted to C++11 or newer, our build system did that when using libc++. Other build systems may not have, but unfortunately I'm not currently able to get at the data to get a concrete answer.

ldionne accepted this revision as: ldionne.Sep 20 2021, 7:43 AM

I did an internal Apple build and didn't see any issues. So this is OK from Apple's internal code base perspective.

However, it doesn't mean that this will be okay for users of libc++, whose ABIs could conceivably leak the mangling of std::nullptr_t. I'd still like to get input from other vendors before making this move. Accepting as myself only to record that this is OK for Apple.

@dexonsmith has had useful views on ABI changes in the past, I'm curious to have his opinion on this change, since I'm still ambivalent.

TL;DR: Today, std::nullptr_t is a different type in C++03 and C++11, which means that the ABI breaks between C++03 and C++11. This patch breaks the ABI in C++03 mode once, right now, to reconcile the C++03 and C++11 ABIs and remove some convolution from libc++. My hunch is that this will not affect too many people and hence it's reasonable. Remember that nullptr is a C++11 construct, so it would have to be someone compiling their code in C++03 mode *and* using our nullptr extension, *and* leaking nullptr_t in their ABI. Do you have an opinion?