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

Repository: rG LLVM Github Monorepo

Unit TestsFailed
View All

	Time	Test
	890 ms	libcxx 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 ms	libcxx 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 ms	libcxx 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 ms	libcxx 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 ms	libcxx 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

Add missing includes found by CI

Harbormaster completed remote builds in B231887: Diff 522032.Sun, May 14, 9:42 PM

Add missing includes found by CI

Add missing includes found by CI

Harbormaster completed remote builds in B231911: Diff 522057.Sun, May 14, 11:46 PM

Add missing includes found by CI

Harbormaster completed remote builds in B232044: Diff 522240.Mon, May 15, 11:39 AM

Add missing includes found by CI

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.

Harbormaster completed remote builds in B232162: Diff 522394.Mon, May 15, 8:18 PM

Add missing includes found by CI

Add missing includes found by CI

Harbormaster completed remote builds in B232198: Diff 522443.Tue, May 16, 2:58 AM

Add missing includes found by CI

Add missing includes found by CI

Harbormaster completed remote builds in B232400: Diff 522742.Tue, May 16, 3:20 PM

Add missing includes found by CI

jansvoboda11 removed a subscriber: jansvoboda11.Tue, May 16, 5:15 PM

Harbormaster completed remote builds in B232460: Diff 522830.Tue, May 16, 5:54 PM

Add missing includes found by CI

Harbormaster completed remote builds in B232499: Diff 522896.Wed, May 17, 3:18 AM

Add missing includes found by CI

In D144322#4344301, @iana wrote:

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).

Harbormaster completed remote builds in B232642: Diff 523095.Wed, May 17, 6:30 PM