Page MenuHomePhabricator

[libc++] Mark some variables as inline, per the spec
AbandonedPublic

Authored by ldionne on Feb 10 2023, 5:00 PM.

Details

Reviewers
Mordante
Group Reviewers
Restricted Project
Summary

This is really what D110243 should have done. These variables are
inline in the spec. We unfortunately need to support pre-C++17 since
some of these inline variables are in C++17 parts of the library that
we provide pre-C++17 as an extension.

Diff Detail

Unit TestsFailed

TimeTest
1,500,110 mslibcxx CI Apple back-deployment macosx10.15 > apple-libc++-backdeployment-cfg-in.std/thread/thread_condition::notify_all_at_thread_exit_lwg3343.pass.cpp
Script: -- : 'COMPILED WITH'; /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++ /Users/libcxx-buildkite-agent/libcxx.buildkite-agent/builds/y10-8-macminivault-com/llvm-project/libcxx-ci/libcxx/test/std/thread/thread.condition/notify_all_at_thread_exit_lwg3343.pass.cpp -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.3.sdk --target=x86_64-apple-macosx10.15 -nostdinc++ -I /Users/libcxx-buildkite-agent/libcxx.buildkite-agent/builds/y10-8-macminivault-com/llvm-project/libcxx-ci/build/apple-system-backdeployment-10.15/include/c++/v1 -I /Users/libcxx-buildkite-agent/libcxx.buildkite-agent/builds/y10-8-macminivault-com/llvm-project/libcxx-ci/libcxx/test/support -std=c++2b -Werror -Wall -Wctad-maybe-unsupported -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 -Wno-tautological-compare -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -Werror=thread-safety -Wuser-defined-warnings -nostdlib++ -L /Users/libcxx-buildkite-agent/libcxx.buildkite-agent/builds/y10-8-macminivault-com/llvm-project/libcxx-ci/build/apple-system-backdeployment-10.15/lib -lc++ -o /Users/libcxx-buildkite-agent/libcxx.buildkite-agent/builds/y10-8-macminivault-com/llvm-project/libcxx-ci/build/apple-system-backdeployment-10.15/test/std/thread/thread.condition/Output/notify_all_at_thread_exit_lwg3343.pass.cpp.dir/t.tmp.exe
1,500,040 mslibcxx CI Apple back-deployment macosx11.0 arm64 > apple-libc++-backdeployment-cfg-in.std/thread/thread_condition::notify_all_at_thread_exit_lwg3343.pass.cpp
Script: -- : 'COMPILED WITH'; /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++ /Users/libcxx-buildkite-agent/libcxx.buildkite-agent/builds/w4-4macminivaultcom-local/llvm-project/libcxx-ci/libcxx/test/std/thread/thread.condition/notify_all_at_thread_exit_lwg3343.pass.cpp -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.3.sdk --target=arm64-apple-macosx11.0 -nostdinc++ -I /Users/libcxx-buildkite-agent/libcxx.buildkite-agent/builds/w4-4macminivaultcom-local/llvm-project/libcxx-ci/build/apple-system-backdeployment-11.0/include/c++/v1 -I /Users/libcxx-buildkite-agent/libcxx.buildkite-agent/builds/w4-4macminivaultcom-local/llvm-project/libcxx-ci/libcxx/test/support -std=c++2b -Werror -Wall -Wctad-maybe-unsupported -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 -Wno-tautological-compare -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -Werror=thread-safety -Wuser-defined-warnings -nostdlib++ -L /Users/libcxx-buildkite-agent/libcxx.buildkite-agent/builds/w4-4macminivaultcom-local/llvm-project/libcxx-ci/build/apple-system-backdeployment-11.0/lib -lc++ -o /Users/libcxx-buildkite-agent/libcxx.buildkite-agent/builds/w4-4macminivaultcom-local/llvm-project/libcxx-ci/build/apple-system-backdeployment-11.0/test/std/thread/thread.condition/Output/notify_all_at_thread_exit_lwg3343.pass.cpp.dir/t.tmp.exe
2,060 mslibcxx CI Clang-cl (Static) > llvm-libc++-static-clangcl-cfg-in.libcxx/experimental/memory/memory_polymorphic_allocator_class/memory_polymorphic_allocator_mem::construct_piecewise_pair.pass.cpp
Script: -- : 'COMPILED WITH'; 'C:/Program Files/LLVM/bin/clang-cl.exe' C:\ws\w8\llvm-project\libcxx-ci\libcxx\test\libcxx\experimental\memory\memory.polymorphic.allocator.class\memory.polymorphic.allocator.mem\construct_piecewise_pair.pass.cpp --driver-mode=g++ --target=x86_64-pc-windows-msvc -nostdinc++ -I C:/ws/w8/llvm-project/libcxx-ci/build/clang-cl-static/include/c++/v1 -I C:/ws/w8/llvm-project/libcxx-ci/build/clang-cl-static/include/c++/v1 -I C:/ws/w8/llvm-project/libcxx-ci/libcxx/test/support -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_STDIO_ISO_WIDE_SPECIFIERS -DNOMINMAX -std=c++2b -Werror -Wall -Wctad-maybe-unsupported -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 -Wno-tautological-compare -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D_LIBCPP_ENABLE_EXPERIMENTAL -D_LIBCPP_DISABLE_AVAILABILITY -Werror=thread-safety -Wuser-defined-warnings -D_LIBCPP_DISABLE_DEPRECATION_WARNINGS -llibc++experimental -nostdlib -L C:/ws/w8/llvm-project/libcxx-ci/build/clang-cl-static/lib -llibc++ -lmsvcrt -lmsvcprt -loldnames -o C:\ws\w8\llvm-project\libcxx-ci\build\clang-cl-static\test\libcxx\experimental\memory\memory.polymorphic.allocator.class\memory.polymorphic.allocator.mem\Output\construct_piecewise_pair.pass.cpp.dir\t.tmp.exe
1,660 mslibcxx CI Clang-cl (Static) > llvm-libc++-static-clangcl-cfg-in.libcxx/thread/thread_mutex::thread_safety_annotations_not_enabled.pass.cpp
Script: -- : 'COMPILED WITH'; 'C:/Program Files/LLVM/bin/clang-cl.exe' C:\ws\w8\llvm-project\libcxx-ci\libcxx\test\libcxx\thread\thread.mutex\thread_safety_annotations_not_enabled.pass.cpp --driver-mode=g++ --target=x86_64-pc-windows-msvc -nostdinc++ -I C:/ws/w8/llvm-project/libcxx-ci/build/clang-cl-static/include/c++/v1 -I C:/ws/w8/llvm-project/libcxx-ci/build/clang-cl-static/include/c++/v1 -I C:/ws/w8/llvm-project/libcxx-ci/libcxx/test/support -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_STDIO_ISO_WIDE_SPECIFIERS -DNOMINMAX -std=c++2b -Werror -Wall -Wctad-maybe-unsupported -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 -Wno-tautological-compare -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D_LIBCPP_ENABLE_EXPERIMENTAL -D_LIBCPP_DISABLE_AVAILABILITY -Werror=thread-safety -Wuser-defined-warnings -llibc++experimental -nostdlib -L C:/ws/w8/llvm-project/libcxx-ci/build/clang-cl-static/lib -llibc++ -lmsvcrt -lmsvcprt -loldnames -o C:\ws\w8\llvm-project\libcxx-ci\build\clang-cl-static\test\libcxx\thread\thread.mutex\Output\thread_safety_annotations_not_enabled.pass.cpp.dir\t.tmp.exe
1,970 mslibcxx CI Clang-cl (Static) > llvm-libc++-static-clangcl-cfg-in.libcxx/utilities/utility/mem_res/mem_poly_allocator_class/mem_poly_allocator_mem::construct_piecewise_pair.pass.cpp
Script: -- : 'COMPILED WITH'; 'C:/Program Files/LLVM/bin/clang-cl.exe' C:\ws\w8\llvm-project\libcxx-ci\libcxx\test\libcxx\utilities\utility\mem.res\mem.poly.allocator.class\mem.poly.allocator.mem\construct_piecewise_pair.pass.cpp --driver-mode=g++ --target=x86_64-pc-windows-msvc -nostdinc++ -I C:/ws/w8/llvm-project/libcxx-ci/build/clang-cl-static/include/c++/v1 -I C:/ws/w8/llvm-project/libcxx-ci/build/clang-cl-static/include/c++/v1 -I C:/ws/w8/llvm-project/libcxx-ci/libcxx/test/support -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_STDIO_ISO_WIDE_SPECIFIERS -DNOMINMAX -std=c++2b -Werror -Wall -Wctad-maybe-unsupported -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 -Wno-tautological-compare -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D_LIBCPP_ENABLE_EXPERIMENTAL -D_LIBCPP_DISABLE_AVAILABILITY -Werror=thread-safety -Wuser-defined-warnings -llibc++experimental -nostdlib -L C:/ws/w8/llvm-project/libcxx-ci/build/clang-cl-static/lib -llibc++ -lmsvcrt -lmsvcprt -loldnames -o C:\ws\w8\llvm-project\libcxx-ci\build\clang-cl-static\test\libcxx\utilities\utility\mem.res\mem.poly.allocator.class\mem.poly.allocator.mem\Output\construct_piecewise_pair.pass.cpp.dir\t.tmp.exe
View Full Test Results (81 Failed)

Event Timeline

ldionne created this revision.Feb 10 2023, 5:00 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 10 2023, 5:00 PM
ldionne requested review of this revision.Feb 10 2023, 5:00 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 10 2023, 5:00 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript

Thanks for working on this. LGTM, but I would like to bikeshed regarding the name.

libcxx/include/__config
889

I wonder should we use this instead? And just constexpr in the #else. For me that makes it easier to understand what the macro does.

ldionne marked an inline comment as done.Feb 11 2023, 8:36 AM
ldionne added inline comments.
libcxx/include/__config
889

The problem is that we still want those variables to be constexpr even in pre C++17.

Mordante added inline comments.Feb 12 2023, 10:24 AM
libcxx/include/__config
889

Yes therefore I suggested "And just constexpr in the #else". With our attributes we often write the "ideal" version for a certain C++ version and a work-around for older versions.

ldionne updated this revision to Diff 496950.Feb 13 2023, 6:09 AM
ldionne marked an inline comment as done.

Address comments.

Mordante accepted this revision.Feb 13 2023, 10:32 AM

LGTM!

libcxx/include/__config
891

I really like to comment!

This revision is now accepted and ready to land.Feb 13 2023, 10:32 AM
ldionne abandoned this revision.Wed, Mar 8, 8:17 AM

It turns out that this is more complicated than it seems. I am going to abandon this in favour of the stack of patches starting at D145422.