Page MenuHomePhabricator

[libc++][test] Improves locale validation.
Needs ReviewPublic

Authored by Mordante on Jan 22 2023, 9:51 AM.

Details

Reviewers
ldionne
Group Reviewers
Restricted Project
Summary

This makes it possible to query the status of a locale in a test. This
allows to use a locale conditionally instead of disabling an entire test
when a locale is missing.

The change is only applied to tests that were disabled in
https://reviews.llvm.org/rGd819703410c2362cbafb60117dab45b182d8b13d

Diff Detail

Unit TestsFailed

TimeTest
1,500,160 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 -fcoroutines-ts -Werror=thread-safety -Wuser-defined-warnings -I /Users/libcxx-buildkite-agent/libcxx.buildkite-agent/builds/y10-8-macminivault-com/llvm-project/libcxx-ci/build/apple-system-backdeployment-10.15/test/support -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 with assertions enabled > 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/f1-1-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=arm64-apple-macosx11.0 -nostdinc++ -I /Users/libcxx-buildkite-agent/libcxx.buildkite-agent/builds/f1-1-macminivault-com/llvm-project/libcxx-ci/build/apple-system-backdeployment-assertions-11.0/include/c++/v1 -I /Users/libcxx-buildkite-agent/libcxx.buildkite-agent/builds/f1-1-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 -D_LIBCPP_ENABLE_ASSERTIONS=1 -fcoroutines-ts -Werror=thread-safety -Wuser-defined-warnings -I /Users/libcxx-buildkite-agent/libcxx.buildkite-agent/builds/f1-1-macminivault-com/llvm-project/libcxx-ci/build/apple-system-backdeployment-assertions-11.0/test/support -nostdlib++ -L /Users/libcxx-buildkite-agent/libcxx.buildkite-agent/builds/f1-1-macminivault-com/llvm-project/libcxx-ci/build/apple-system-backdeployment-assertions-11.0/lib -lc++ -o /Users/libcxx-buildkite-agent/libcxx.buildkite-agent/builds/f1-1-macminivault-com/llvm-project/libcxx-ci/build/apple-system-backdeployment-assertions-11.0/test/std/thread/thread.condition/Output/notify_all_at_thread_exit_lwg3343.pass.cpp.dir/t.tmp.exe
35,250 mslibcxx CI Clang-cl (Static) > llvm-libc++-static-clangcl-cfg-in.std/thread/futures/futures_async::async.pass.cpp
Script: -- : 'COMPILED WITH'; 'C:/Program Files/LLVM/bin/clang-cl.exe' C:\ws\w5\llvm-project\libcxx-ci\libcxx\test\std\thread\futures\futures.async\async.pass.cpp --driver-mode=g++ --target=x86_64-pc-windows-msvc -nostdinc++ -I C:/ws/w5/llvm-project/libcxx-ci/build/clang-cl-static/include/c++/v1 -I C:/ws/w5/llvm-project/libcxx-ci/build/clang-cl-static/include/c++/v1 -I C:/ws/w5/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 -fcoroutines-ts -Werror=thread-safety -Wuser-defined-warnings -I C:/ws/w5/llvm-project/libcxx-ci/build/clang-cl-static\test\support -llibc++experimental -nostdlib -L C:/ws/w5/llvm-project/libcxx-ci/build/clang-cl-static/lib -llibc++ -lmsvcrt -lmsvcprt -loldnames -o C:\ws\w5\llvm-project\libcxx-ci\build\clang-cl-static\test\std\thread\futures\futures.async\Output\async.pass.cpp.dir\t.tmp.exe
16,470 mslibcxx CI Clang-cl (Static) > llvm-libc++-static-clangcl-cfg-in.std/thread/thread_mutex/thread_mutex_requirements/thread_sharedtimedmutex_requirements/thread_sharedtimedmutex_class::try_lock_shared_for.pass.cpp
Script: -- : 'COMPILED WITH'; 'C:/Program Files/LLVM/bin/clang-cl.exe' C:\ws\w5\llvm-project\libcxx-ci\libcxx\test\std\thread\thread.mutex\thread.mutex.requirements\thread.sharedtimedmutex.requirements\thread.sharedtimedmutex.class\try_lock_shared_for.pass.cpp --driver-mode=g++ --target=x86_64-pc-windows-msvc -nostdinc++ -I C:/ws/w5/llvm-project/libcxx-ci/build/clang-cl-static/include/c++/v1 -I C:/ws/w5/llvm-project/libcxx-ci/build/clang-cl-static/include/c++/v1 -I C:/ws/w5/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 -fcoroutines-ts -Werror=thread-safety -Wuser-defined-warnings -I C:/ws/w5/llvm-project/libcxx-ci/build/clang-cl-static\test\support -llibc++experimental -nostdlib -L C:/ws/w5/llvm-project/libcxx-ci/build/clang-cl-static/lib -llibc++ -lmsvcrt -lmsvcprt -loldnames -o C:\ws\w5\llvm-project\libcxx-ci\build\clang-cl-static\test\std\thread\thread.mutex\thread.mutex.requirements\thread.sharedtimedmutex.requirements\thread.sharedtimedmutex.class\Output\try_lock_shared_for.pass.cpp.dir\t.tmp.exe

Event Timeline

Mordante created this revision.Jan 22 2023, 9:51 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 22 2023, 9:51 AM
Herald added a subscriber: arichardson. · View Herald Transcript
Mordante requested review of this revision.Jan 22 2023, 9:51 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 22 2023, 9:51 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript

I think this makes sense. However, I would avoid adding new compiler flags since we try to keep our command-lines as minimal as possible in the test suite.

Instead, I would suggest creating a general mechanism for generating a test_config.h header (which would be the test-suite equivalent of __config_site) and defining those macros there. This header would be generated at lit configuration time (not at CMake configuration time like the __config_site header). This mechanism would be a bit more general and I think it could even be used for other purposes, for example glibc-old-ru_RU-decimal-point could probably be removed in favor of a macro, and we'd gain test coverage in libcxx/test/std/localization/locale.categories/category.monetary/locale.money.put/locale.money.put.members/put_long_double_ru_RU.pass.cpp.

We don't have a precedent for doing this in our test suite, so we'll have to design a way to do it.

Mordante updated this revision to Diff 492148.Jan 25 2023, 9:12 AM

Try a different approach as discussed during review.

Note this is still a proof-of-concept version not a clean patch.

libcxx/test/std/containers/container.adaptors/container.adaptors.format/format.functions.tests.h
607

Debug to validate the changes work locally.

libcxx/utils/libcxx/test/dsl.py
524

This is a bit hacky to add the path to the includes. I don't want to pollute the source directory with this file.

arichardson added inline comments.Jan 26 2023, 12:00 AM
libcxx/test/std/utilities/format/format.range/format.range.formatter/format.functions.tests.h
590

How about generating a header with these macros set to 0/1, then typos could be caught by -Werror=undef?

Mordante added inline comments.Jan 30 2023, 10:56 AM
libcxx/test/std/utilities/format/format.range/format.range.formatter/format.functions.tests.h
590

Typically we use a different way to define these macros

#  ifndef TEST_HAS_NO_LOCALE_fr_FR_UTF_8

I just didn't do that yet to discuss whether this basic approach is feasible.
Then the macro doesn't need 0/1.

Thanks for looking into this! I am not a huge fan of using the config object to keep state around, but I understand why you had to do it. I'll look into alternative solutions, please give me a bit of time -- let's talk about this again after Issaquah.

libcxx/test/support/test_macros.h
25

I would call the header test_config_site.h.

26–30

I would include this unconditionally. If our setup is messed up and there's no config_site.h, we want to know about it instead of silently having all these macros be undefined.

libcxx/utils/libcxx/test/dsl.py
508–514