Page MenuHomePhabricator

[libc++][Modules] Make top level modules for all C++ headers with OS/clang versions
Needs ReviewPublic

Authored by iana on Feb 18 2023, 12:14 AM.

Details

Reviewers
Bigcheese
ldionne
jdoerfert
Mordante
dexonsmith
vsapsai
Group Reviewers
Restricted Project
Summary

The headers that include_next compiler and OS headers need to be in different top level modules in order to avoid module cycles. e.g. libc++'s stdlib.h will #include_next stdlib.h from the compiler and then the C library. Either of those are likely to include stddef.h, which will come back up to the libc++ module map and create a module cycle. Putting stdlib.h and stddef.h (and the rest of the C standard library headers) in top level modules resolves this by letting the order go cxx_stdlib_h -> os_stdlib_h -> cxx_stddef_h -> os_stddef_h.

All of those headers' dependencies then need to be moved into top level modules themselves to avoid module cycles between the new top level level cstd modules. This starts to get complicated, as the libc++ C headers, by standard, have to include many of the C++ headers, which include the private detail headers, which are intertwined. e.g. some __algorithm headers include __memory headers and vice versa.

Make top level modules for all of the public libc++ headers, and add a script to generate modules for all of the private detail headers such that the modules aren't cyclic.

Diff Detail

Unit TestsFailed

TimeTest
890 mslibcxx CI - Modular build > llvm-libc++-shared-cfg-in.libcxx/input_output/filesystems/class_directory_entry/directory_entry_mods::last_write_time.pass.cpp
Script: -- : 'COMPILED WITH'; /usr/bin/clang++-17 /home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-16c0e047963c-1/llvm-project/libcxx-ci/libcxx/test/libcxx/input.output/filesystems/class.directory_entry/directory_entry.mods/last_write_time.pass.cpp -pthread --target=x86_64-unknown-linux-gnu -nostdinc++ -I /home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-16c0e047963c-1/llvm-project/libcxx-ci/build/generic-modules/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-16c0e047963c-1/llvm-project/libcxx-ci/build/generic-modules/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-16c0e047963c-1/llvm-project/libcxx-ci/libcxx/test/support -std=c++26 -fmodules -fcxx-modules -Werror -Wall -Wctad-maybe-unsupported -Wextra -Wshadow -Wundef -Wunused-template -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 -Werror=thread-safety -Wuser-defined-warnings -I /home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-16c0e047963c-1/llvm-project/libcxx-ci/libcxx/test/libcxx/input.output/filesystems/class.directory_entry/directory_entry.mods/../../../../../../src/filesystem -lc++experimental -nostdlib++ -L /home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-16c0e047963c-1/llvm-project/libcxx-ci/build/generic-modules/lib -Wl,-rpath,/home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-16c0e047963c-1/llvm-project/libcxx-ci/build/generic-modules/lib -lc++ -o /home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-16c0e047963c-1/llvm-project/libcxx-ci/build/generic-modules/test/libcxx/input.output/filesystems/class.directory_entry/directory_entry.mods/Output/last_write_time.pass.cpp.dir/t.
140 mslibcxx CI - Modular build > llvm-libc++-shared-cfg-in.libcxx/strings/char_traits/char_traits_specializations::arbitrary_char_type.deprecated.verify.cpp
Script: -- : 'COMPILED WITH'; /usr/bin/clang++-17 /home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-16c0e047963c-1/llvm-project/libcxx-ci/libcxx/test/libcxx/strings/char.traits/char.traits.specializations/arbitrary_char_type.deprecated.verify.cpp -pthread --target=x86_64-unknown-linux-gnu -nostdinc++ -I /home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-16c0e047963c-1/llvm-project/libcxx-ci/build/generic-modules/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-16c0e047963c-1/llvm-project/libcxx-ci/build/generic-modules/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-16c0e047963c-1/llvm-project/libcxx-ci/libcxx/test/support -std=c++26 -fmodules -fcxx-modules -Werror -Wall -Wctad-maybe-unsupported -Wextra -Wshadow -Wundef -Wunused-template -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 -Werror=thread-safety -Wuser-defined-warnings -fsyntax-only -Wno-error -Xclang -verify -Xclang -verify-ignore-unexpected=note -ferror-limit=0
1,210 mslibcxx CI - Modular build > llvm-libc++-shared-cfg-in.std/input_output/filesystems/class_directory_entry/directory_entry_cons::copy.pass.cpp
Script: -- : 'COMPILED WITH'; /usr/bin/clang++-17 /home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-16c0e047963c-1/llvm-project/libcxx-ci/libcxx/test/std/input.output/filesystems/class.directory_entry/directory_entry.cons/copy.pass.cpp -pthread --target=x86_64-unknown-linux-gnu -nostdinc++ -I /home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-16c0e047963c-1/llvm-project/libcxx-ci/build/generic-modules/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-16c0e047963c-1/llvm-project/libcxx-ci/build/generic-modules/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-16c0e047963c-1/llvm-project/libcxx-ci/libcxx/test/support -std=c++26 -fmodules -fcxx-modules -Werror -Wall -Wctad-maybe-unsupported -Wextra -Wshadow -Wundef -Wunused-template -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 -Werror=thread-safety -Wuser-defined-warnings -lc++experimental -nostdlib++ -L /home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-16c0e047963c-1/llvm-project/libcxx-ci/build/generic-modules/lib -Wl,-rpath,/home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-16c0e047963c-1/llvm-project/libcxx-ci/build/generic-modules/lib -lc++ -o /home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-16c0e047963c-1/llvm-project/libcxx-ci/build/generic-modules/test/std/input.output/filesystems/class.directory_entry/directory_entry.cons/Output/copy.pass.cpp.dir/t.tmp.exe
1,050 mslibcxx CI - Modular build > llvm-libc++-shared-cfg-in.std/input_output/filesystems/class_directory_entry/directory_entry_cons::copy_assign.pass.cpp
Script: -- : 'COMPILED WITH'; /usr/bin/clang++-17 /home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-16c0e047963c-1/llvm-project/libcxx-ci/libcxx/test/std/input.output/filesystems/class.directory_entry/directory_entry.cons/copy_assign.pass.cpp -pthread --target=x86_64-unknown-linux-gnu -nostdinc++ -I /home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-16c0e047963c-1/llvm-project/libcxx-ci/build/generic-modules/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-16c0e047963c-1/llvm-project/libcxx-ci/build/generic-modules/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-16c0e047963c-1/llvm-project/libcxx-ci/libcxx/test/support -std=c++26 -fmodules -fcxx-modules -Werror -Wall -Wctad-maybe-unsupported -Wextra -Wshadow -Wundef -Wunused-template -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 -Werror=thread-safety -Wuser-defined-warnings -lc++experimental -nostdlib++ -L /home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-16c0e047963c-1/llvm-project/libcxx-ci/build/generic-modules/lib -Wl,-rpath,/home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-16c0e047963c-1/llvm-project/libcxx-ci/build/generic-modules/lib -lc++ -o /home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-16c0e047963c-1/llvm-project/libcxx-ci/build/generic-modules/test/std/input.output/filesystems/class.directory_entry/directory_entry.cons/Output/copy_assign.pass.cpp.dir/t.tmp.exe
1,180 mslibcxx CI - Modular build > llvm-libc++-shared-cfg-in.std/input_output/filesystems/class_directory_entry/directory_entry_cons::move.pass.cpp
Script: -- : 'COMPILED WITH'; /usr/bin/clang++-17 /home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-16c0e047963c-1/llvm-project/libcxx-ci/libcxx/test/std/input.output/filesystems/class.directory_entry/directory_entry.cons/move.pass.cpp -pthread --target=x86_64-unknown-linux-gnu -nostdinc++ -I /home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-16c0e047963c-1/llvm-project/libcxx-ci/build/generic-modules/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-16c0e047963c-1/llvm-project/libcxx-ci/build/generic-modules/include/c++/v1 -I /home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-16c0e047963c-1/llvm-project/libcxx-ci/libcxx/test/support -std=c++26 -fmodules -fcxx-modules -Werror -Wall -Wctad-maybe-unsupported -Wextra -Wshadow -Wundef -Wunused-template -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 -Werror=thread-safety -Wuser-defined-warnings -lc++experimental -nostdlib++ -L /home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-16c0e047963c-1/llvm-project/libcxx-ci/build/generic-modules/lib -Wl,-rpath,/home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-16c0e047963c-1/llvm-project/libcxx-ci/build/generic-modules/lib -lc++ -o /home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-16c0e047963c-1/llvm-project/libcxx-ci/build/generic-modules/test/std/input.output/filesystems/class.directory_entry/directory_entry.cons/Output/move.pass.cpp.dir/t.tmp.exe
View Full Test Results (93 Failed)

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
iana updated this revision to Diff 522032.Sun, May 14, 9:22 PM
  • Add missing includes found by CI
iana updated this revision to Diff 522042.Sun, May 14, 10:00 PM
  • Add missing includes found by CI
iana updated this revision to Diff 522057.Sun, May 14, 11:26 PM
  • Add missing includes found by CI
iana updated this revision to Diff 522240.Mon, May 15, 9:52 AM
  • Add missing includes found by CI
iana updated this revision to Diff 522341.Mon, May 15, 2:17 PM
  • Add missing includes found by CI
iana updated this revision to Diff 522394.Mon, May 15, 5:36 PM
  • Add missing includes found by CI
iana planned changes to this revision.Mon, May 15, 5:37 PM

If you want to ignore all the added includes and review the rest, it should be in a good state. I'm temporarily using this review to test for more missing includes via CI. I guess my local environment is different enough that it didn't find all of them.

iana updated this revision to Diff 522421.Mon, May 15, 8:50 PM
  • Add missing includes found by CI
iana updated this revision to Diff 522443.Mon, May 15, 10:47 PM
  • Add missing includes found by CI
iana updated this revision to Diff 522700.Tue, May 16, 10:40 AM
  • Add missing includes found by CI
iana updated this revision to Diff 522742.Tue, May 16, 12:34 PM
  • Add missing includes found by CI
iana updated this revision to Diff 522830.Tue, May 16, 4:45 PM
  • Add missing includes found by CI
iana updated this revision to Diff 522896.Tue, May 16, 10:17 PM
  • Add missing includes found by CI
iana updated this revision to Diff 523095.Wed, May 17, 10:10 AM
  • Add missing includes found by CI

If you want to ignore all the added includes and review the rest, it should be in a good state. I'm temporarily using this review to test for more missing includes via CI. I guess my local environment is different enough that it didn't find all of them.

I wonder if it'd be easy to look at the log from the bot that's failing, and use that CMake configuration locally to find these with a faster turnaround. For example, a lot of the fixes seem to be in libcxx/include/pstl/; perhaps there's a configuration to turn on (more of?) the PSTL locally.

Also, in case you didn't know, there's a way to link two reviews together in Phabricator: in the top-right, "Edit Related Revisions". You could put these include fixes in a "parent" revision (or in a bunch of parent revisions), keeping this review clean, and manually trigger a new bot run / rebase each time you update the parent with more fixes (or add a parent).

iana updated this revision to Diff 523258.Wed, May 17, 9:52 PM
  • Add missing includes found by CI
iana updated this revision to Diff 523450.Thu, May 18, 10:48 AM
  • Add missing includes found by CI
iana updated this revision to Diff 523528.Thu, May 18, 1:07 PM
  • Add missing includes found by CI
iana updated this revision to Diff 523651.Thu, May 18, 9:45 PM
  • Add missing includes found by CI
iana updated this revision to Diff 523903.Fri, May 19, 12:56 PM
  • Add missing includes found by CI
iana updated this revision to Diff 523994.Fri, May 19, 10:59 PM
  • Add missing includes found by CI
iana updated this revision to Diff 524051.Sat, May 20, 1:07 PM
  • Add missing includes found by CI
iana updated this revision to Diff 524060.Sat, May 20, 3:03 PM
  • Add missing includes found by CI
iana updated this revision to Diff 524065.Sat, May 20, 3:33 PM
  • Add missing includes found by CI
iana updated this revision to Diff 524075.Sat, May 20, 9:10 PM
  • Add missing includes found by CI
iana updated this revision to Diff 524077.Sat, May 20, 10:11 PM
  • Add missing includes found by CI
iana updated this revision to Diff 524078.Sat, May 20, 10:41 PM
  • Add missing includes found by CI
iana updated this revision to Diff 524081.Sat, May 20, 11:43 PM
  • Add missing includes found by CI
iana updated this revision to Diff 524416.Mon, May 22, 11:58 AM
  • Add missing includes found by CI
iana updated this revision to Diff 524545.Mon, May 22, 5:45 PM
  • Add missing includes found by CI
iana updated this revision to Diff 524569.Mon, May 22, 10:57 PM
  • Add missing includes found by CI
iana updated this revision to Diff 524803.Tue, May 23, 10:57 AM
  • Add missing includes found by CI
iana updated this revision to Diff 525009.Tue, May 23, 10:35 PM
  • Add missing includes found by CI
iana updated this revision to Diff 525264.Wed, May 24, 10:52 AM
  • Add missing includes found by CI
iana updated this revision to Diff 525447.Wed, May 24, 10:57 PM
  • Add missing includes found by CI