This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Use the using_if_exists attribute when provided
ClosedPublic

Authored by ldionne on Oct 27 2020, 11:23 AM.

Details

Summary

As discussed on cfe-dev [1], use the using_if_exists Clang attribute when
the compiler supports it. This makes it easier to port libc++ on top of
new platforms that don't fully support the C Standard library.

Previously, libc++ would fail to build when trying to import a missing
declaration in a <cXXXX> header. With the attribute, the declaration will
simply not be imported into namespace std, and hence it won't be available
for libc++ to use. In many cases, the declarations were *not* actually
required for libc++ to work (they were only surfaced for users to use
them as std::XXXX), so not importing them into namespace std is acceptable.

The same thing could be achieved by conscious usage of #ifdef along
with platform detection, however that quickly creates a maintenance
problem as libc++ is ported to new platforms. Furthermore, this problem
is exacerbated when mixed with vendor internal-only platforms, which can
lead to difficulties maintaining a downstream fork of the library.

For the time being, we only use the using_if_exists attribute when it
is supported. At some point in the future, we will start removing #ifdef
paths that are unnecessary when the attribute is supported, and folks
who need those #ifdef paths will be required to use a compiler that
supports the attribute.

[1]: http://lists.llvm.org/pipermail/cfe-dev/2020-June/066038.html

Diff Detail

Event Timeline

ldionne created this revision.Oct 27 2020, 11:23 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 27 2020, 11:23 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
ldionne requested review of this revision.Oct 27 2020, 11:23 AM

Note that once this is in, we can also start dropping existing #ifs in the headers to workaround various declarations missing in the underlying C stdlib, as long as we require a sufficiently recent Clang.

Note that once this is in, we can also start dropping existing #ifs in the headers to workaround various declarations missing in the underlying C stdlib, as long as we require a sufficiently recent Clang.

What about other compilers?

Note that once this is in, we can also start dropping existing #ifs in the headers to workaround various declarations missing in the underlying C stdlib, as long as we require a sufficiently recent Clang.

What about other compilers?

We're not there yet -- I was just pointing to a possible future cleanup.

For other compilers: it seems reasonable to mandate that the attribute is supported on the compiler you're using if you're trying to port libc++ to a platform that doesn't provide a full standard library. It's a balance between ease-of-porting and ease-of-maintaining.

ldionne updated this revision to Diff 349293.Jun 2 2021, 8:57 AM
ldionne edited the summary of this revision. (Show Details)

Rebase onto main since the attribute has landed.

After a quick grep through the sources, it seems like the following macros are the ones that currently gate using declarations:

_LIBCPP_C_HAS_NO_GETS
_LIBCPP_HAS_ALIGNED_ALLOC
_LIBCPP_HAS_NO_FGETPOS_FSETPOS
_LIBCPP_HAS_NO_GLOBAL_FILESYSTEM_NAMESPACE
_LIBCPP_HAS_NO_LONG_LONG
_LIBCPP_HAS_NO_STDIN
_LIBCPP_HAS_NO_STDOUT
_LIBCPP_HAS_NO_THREAD_UNSAFE_C_FUNCTIONS
_LIBCPP_HAS_QUICK_EXIT
_LIBCPP_HAS_TIMESPEC_GET
_LIBCPP_WINDOWS_STORE_APP

If you see this review and are currently depending on one of these macros, please be aware that we will eventually stop guarding those declarations based on those macros (and in some cases that means a macro will become unused and we'll remove it). I'll do my best to communicate with vendors before that happens, however this is an early heads up.

@jasonliu @lebedev.ri @mstorsjo @danalbert @phosek for awareness.

ldionne updated this revision to Diff 349540.Jun 3 2021, 7:05 AM

Rebase onto main

ldionne accepted this revision as: Restricted Project.Jun 3 2021, 3:27 PM
ldionne updated this revision to Diff 349713.Jun 3 2021, 3:39 PM

Rebase and add a few usings that had been forgotten.

This revision was not accepted when it landed; it landed in state Needs Review.Jun 4 2021, 6:55 AM
This revision was automatically updated to reflect the committed changes.