Page MenuHomePhabricator

[libc++] Remove _LIBCPP_ALIGNOF
Needs RevisionPublic

Authored by philnik on May 23 2022, 3:39 PM.

Details

Reviewers
ldionne
Mordante
var-const
Group Reviewers
Restricted Project
Summary

We already define a lot of C++11 keywords in C++03, so we can also do that for alignof instead of having an internal macro for that.

Diff Detail

Unit TestsFailed

TimeTest
5,270 mslibcxx CI C++11 > llvm-libc++-shared-cfg-in.libcxx::clang_tidy.sh.cpp
Script: -- : 'RUN: at line 12'; clang-tidy /home/libcxx-builder/.buildkite-agent/builds/df72a9538cc7-1/llvm-project/libcxx-ci/libcxx/test/libcxx/clang_tidy.sh.cpp --warnings-as-errors=* -header-filter=.* -- -Wno-unknown-warning-option -nostdinc++ -I /home/libcxx-builder/.buildkite-agent/builds/df72a9538cc7-1/llvm-project/libcxx-ci/build/generic-cxx11/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/df72a9538cc7-1/llvm-project/libcxx-ci/build/generic-cxx11/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/df72a9538cc7-1/llvm-project/libcxx-ci/libcxx/test/support -std=c++11 -Werror -Wall -Wextra -Wshadow -Wundef -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-c++11-extensions -Wno-noexcept-type -Wno-atomic-alignment -Wno-user-defined-literals -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D_LIBCPP_DISABLE_AVAILABILITY -fcoroutines-ts -Werror=thread-safety -Wuser-defined-warnings
10,850 mslibcxx CI C++2b > llvm-libc++-shared-cfg-in.libcxx::clang_tidy.sh.cpp
Script: -- : 'RUN: at line 12'; clang-tidy /home/libcxx-builder/.buildkite-agent/builds/1af88939fa5d-1/llvm-project/libcxx-ci/libcxx/test/libcxx/clang_tidy.sh.cpp --warnings-as-errors=* -header-filter=.* -- -Wno-unknown-warning-option -nostdinc++ -I /home/libcxx-builder/.buildkite-agent/builds/1af88939fa5d-1/llvm-project/libcxx-ci/build/generic-cxx2b/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/1af88939fa5d-1/llvm-project/libcxx-ci/build/generic-cxx2b/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/1af88939fa5d-1/llvm-project/libcxx-ci/libcxx/test/support -std=c++2b -Werror -Wall -Wextra -Wshadow -Wundef -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-c++11-extensions -Wno-noexcept-type -Wno-atomic-alignment -Wno-user-defined-literals -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D_LIBCPP_DISABLE_AVAILABILITY -fcoroutines-ts -Werror=thread-safety -Wuser-defined-warnings
10,920 mslibcxx CI GCC 11 / C++latest > llvm-libc++-shared-gcc-cfg-in.libcxx::clang_tidy.sh.cpp
Script: -- : 'RUN: at line 12'; clang-tidy /home/libcxx-builder/.buildkite-agent/builds/534abee14b81-1/llvm-project/libcxx-ci/libcxx/test/libcxx/clang_tidy.sh.cpp --warnings-as-errors=* -header-filter=.* -- -Wno-unknown-warning-option -nostdinc++ -I /home/libcxx-builder/.buildkite-agent/builds/534abee14b81-1/llvm-project/libcxx-ci/build/generic-gcc/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/534abee14b81-1/llvm-project/libcxx-ci/build/generic-gcc/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/534abee14b81-1/llvm-project/libcxx-ci/libcxx/test/support -std=c++2b -Werror -Wall -Wextra -Wshadow -Wundef -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-c++11-extensions -Wno-noexcept-type -Wno-aligned-allocation-unavailable -Wno-atomic-alignment -Wno-sized-deallocation -Wno-literal-suffix -Wno-user-defined-literals -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D_LIBCPP_DISABLE_AVAILABILITY -D_LIBCPP_DISABLE_DEPRECATION_WARNINGS -Wno-placement-new -Wno-class-memaccess

Event Timeline

philnik created this revision.May 23 2022, 3:39 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 23 2022, 3:39 PM
philnik requested review of this revision.May 23 2022, 3:39 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 23 2022, 3:39 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
ldionne requested changes to this revision.Fri, May 27, 12:22 PM

As much as I don't like having ugly macros like _LIBCPP_ALIGNOF, I dislike even more defining a keyword as a macro, especially in the library:

  • If the compiler ever wants to support alignof as an extension in C++03, we'll need coordination between the library and the compiler.
  • This is technically a breaking change for C++03 users because they are allowed to use alignof as a name, and we're not allowed to define identifiers outside of our namespace. Concretely, I do suspect this may break some C-first codebases that try to be clever.
  • This breaks the symmetry between _LIBCPP_PREFERRED_ALIGNOF and _LIBCPP_ALIGNOF.
  • We have experience that defining compiler stuff in the library isn't a great idea -- for example, we've yet to figure out a way to get rid of our char16_t typedef, and it's not even a macro.

All in all, I feel that this change doesn't pull its weight, even though I am supportive of the intent. Thoughts?

This revision now requires changes to proceed.Fri, May 27, 12:22 PM
philnik added a comment.EditedSat, May 28, 8:01 AM
  • If the compiler ever wants to support alignof as an extension in C++03, we'll need coordination between the library and the compiler.

That could be avoided by checking for #if defined(_LIBCPP_CXX03_LANG) && !__has_keyword(alignof) instead of just #ifdef _LIBCPP_CXX03_LANG.

  • This is technically a breaking change for C++03 users because they are allowed to use alignof as a name, and we're not allowed to define identifiers outside of our namespace. Concretely, I do suspect this may break some C-first codebases that try to be clever.

C++03 in libc++ is more a crutch than anything else at this point. Users are also allowed to #define a lot of names in C++03 that we use in the library and are only part of C++11.

These are the keywords introduced in C++11 (according to https://en.cppreference.com/w/cpp/keyword):

KeywordProvided?/How?
alignasnot provided
alignofnot provided
char16_ttypedef in libc++
char32_ttypedef in libc++
constexprnot provided
decltypemacro in libc++
noexceptnot provided
nullptrmacro in libc++
static_assertmacro in libc++
thread_localnot provided

So we already define quite a few C++11 keywords. Functionality wise most of them are indistinguishable from their C++11 keyword versions. We can't provide constexpr and noexcept because we don't have a way to emulate them. We don't use thread_local anywhere so I don't particularly care. So effectively the only ones left are alignas and alignof.

I think the C-first codebase that this change would break are very few and are arguably broken already. We would also break these codebases if we included stdalign.h and they are definitely broken by C23 (https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2654.pdf).

  • This breaks the symmetry between _LIBCPP_PREFERRED_ALIGNOF and _LIBCPP_ALIGNOF.

There are exactly 3 uses of _LIBCPP_PREFERRED_ALIGNOF, all of which are in <type_traits>. I think removing the macro and adding a comment on these places is the better alternative.

  • We have experience that defining compiler stuff in the library isn't a great idea -- for example, we've yet to figure out a way to get rid of our char16_t typedef, and it's not even a macro.

I think getting rid of a macro that emulates a keyword is easier than a typedef. The typedef has ABI implications which simply don't exist with these macros.

All in all, I feel that this change doesn't pull its weight, even though I am supportive of the intent. Thoughts?

I know we don't want to break users, but I think we should treat C++03 as what it is: A relic of a bygone era and a burden nobody wants to keep.
It seems like a thing almost nobody uses but nobody knows how many users there are, so we don't want to risk just throwing it away. It's not like it makes sense to have a standard library from 2022 and using it with a language standard from 2003 (or more like 1998). People had over a decade of time to upgrade by now and people who haven't won't use any even nearly up-to date toolchain. Let them deal with it in 20 years when they finally decide to upgrade.

  • If the compiler ever wants to support alignof as an extension in C++03, we'll need coordination between the library and the compiler.

That could be avoided by checking for #if defined(_LIBCPP_CXX03_LANG) && !__has_keyword(alignof) instead of just #ifdef _LIBCPP_CXX03_LANG.

Fair point.

  • This breaks the symmetry between _LIBCPP_PREFERRED_ALIGNOF and _LIBCPP_ALIGNOF.

There are exactly 3 uses of _LIBCPP_PREFERRED_ALIGNOF, all of which are in <type_traits>. I think removing the macro and adding a comment on these places is the better alternative.

Can you upload a patch that does this before we decide whether to move forward with this patch?

I know we don't want to break users, but I think we should treat C++03 as what it is: A relic of a bygone era and a burden nobody wants to keep.

I agree that C++03 is a relic, and I agree that we want to get rid of it. However, I think you are underestimating how many users of it there are. For example, AppleClang still uses C++03 by default today because we've run into issues when flipping the default -- LLVM Clang uses C++14 AFAICT. It doesn't change much w.r.t. this patch, but I think it's important to be aware that a lot of users unfortunately still compile as C++03.

  • This breaks the symmetry between _LIBCPP_PREFERRED_ALIGNOF and _LIBCPP_ALIGNOF.

There are exactly 3 uses of _LIBCPP_PREFERRED_ALIGNOF, all of which are in <type_traits>. I think removing the macro and adding a comment on these places is the better alternative.

Can you upload a patch that does this before we decide whether to move forward with this patch?

This is D127132 now.

I know we don't want to break users, but I think we should treat C++03 as what it is: A relic of a bygone era and a burden nobody wants to keep.

I agree that C++03 is a relic, and I agree that we want to get rid of it. However, I think you are underestimating how many users of it there are. For example, AppleClang still uses C++03 by default today because we've run into issues when flipping the default -- LLVM Clang uses C++14 AFAICT. It doesn't change much w.r.t. this patch, but I think it's important to be aware that a lot of users unfortunately still compile as C++03.

Can you tell me/do you know what problems there were changing the default to something newer than C++03 and when you tried it? I wasn't aware AppleClang still defaults to C++03.
It's absolutely possible that I'm underestimating how many people still compile in C++03 mode. Do you know of any statistics which language mode get used? How many people are just using it because it's the default on AppleClang and how many people would even notice the change? How many people are there that compile in C++03 mode for a good reason (i.e. would have to set -std=c++03 if it weren't the default)? GCC defaults to C++17 and clang to C++14, so it's obviously not impossible to change the default language standard. Is the main reason we still support C++03 that AppleClang still defaults to it?

No real opinion on the patch.

I know we don't want to break users, but I think we should treat C++03 as what it is: A relic of a bygone era and a burden nobody wants to keep.

I agree that C++03 is a relic, and I agree that we want to get rid of it. However, I think you are underestimating how many users of it there are. For example, AppleClang still uses C++03 by default today because we've run into issues when flipping the default -- LLVM Clang uses C++14 AFAICT. It doesn't change much w.r.t. this patch, but I think it's important to be aware that a lot of users unfortunately still compile as C++03.

Can you tell me/do you know what problems there were changing the default to something newer than C++03 and when you tried it? I wasn't aware AppleClang still defaults to C++03.
It's absolutely possible that I'm underestimating how many people still compile in C++03 mode. Do you know of any statistics which language mode get used? How many people are just using it because it's the default on AppleClang and how many people would even notice the change? How many people are there that compile in C++03 mode for a good reason (i.e. would have to set -std=c++03 if it weren't the default)? GCC defaults to C++17 and clang to C++14, so it's obviously not impossible to change the default language standard. Is the main reason we still support C++03 that AppleClang still defaults to it?

There's still code being actively developed in older language Standards, even C++98. Upgrading the newer Standards can be quite a bit of effort. This is even worse when code bases use using namespace std; in headers. (I've seen this in multiple code bases.)

So I fear we'll need to support C++03 for a long time (probably forever).

In last years Jetbrains developer Infographic 12% was still using C++98/03
https://www.jetbrains.com/lp/devecosystem-2021/cpp

Can you tell me/do you know what problems there were changing the default to something newer than C++03 and when you tried it? I wasn't aware AppleClang still defaults to C++03.

I don't know the details because it was before my time, unfortunately.

It's absolutely possible that I'm underestimating how many people still compile in C++03 mode. Do you know of any statistics which language mode get used?

We unfortunately don't have great data around that, but it would be useful for the LLVM community as a whole.

Is the main reason we still support C++03 that AppleClang still defaults to it?

Even if AppleClang switched to C++14 by default, I don't think we'd remove C++03 support unless Clang (so the LLVM community as a whole) decided to also remove support for -std=c++03.

The more I think about this change, the less certain I am about it. If we want to do this, why not pursue providing alignof as an extension in C++03 mode in Clang itself? It seems to me that the better place to bridge language gaps is in the compiler itself.