Page MenuHomePhabricator

[libc++][nfc] Move iterator_traits and related into __iterator/iterator_traits.h.
ClosedPublic

Authored by zoecarver on Apr 16 2021, 2:21 PM.

Details

Summary

Based on D100682 and D99855.

(Note: I originally was going to just make this part of D99855, but I decided not to because this patch moves lots of unrelated code around, and I didn't want to make D99855 harder to review because of unrelated code-changes/moves.)

Diff Detail

Unit TestsFailed

TimeTest
240 mslibcxx CI C++03 > libc++.std/iterators/iterator_primitives/iterator_traits::empty.pass.cpp
Script: -- : 'COMPILED WITH'; /usr/bin/clang++ /home/libcxx-builder/.buildkite-agent/builds/163fa794a6e7-1/llvm-project/libcxx-ci/libcxx/test/std/iterators/iterator.primitives/iterator.traits/empty.pass.cpp -v --target=x86_64-unknown-linux-gnu -include /home/libcxx-builder/.buildkite-agent/builds/163fa794a6e7-1/llvm-project/libcxx-ci/libcxx/test/support/nasty_macros.h -nostdinc++ -I/home/libcxx-builder/.buildkite-agent/builds/163fa794a6e7-1/llvm-project/libcxx-ci/build/generic-cxx03/include/c++/v1 -I/home/libcxx-builder/.buildkite-agent/builds/163fa794a6e7-1/llvm-project/libcxx-ci/build/generic-cxx03/projects/libcxx/include/c++build -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -D__STDC_CONSTANT_MACROS -I/home/libcxx-builder/.buildkite-agent/builds/163fa794a6e7-1/llvm-project/libcxx-ci/libcxx/test/support -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -std=c++03 -Werror -Wall -Wextra -Wshadow -Wundef -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-c++11-extensions -Wno-user-defined-literals -Wno-noexcept-type -Wno-atomic-alignment -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -D_LIBCPP_DISABLE_AVAILABILITY -fcoroutines-ts -Werror=thread-safety -Wuser-defined-warnings -fmodules-cache-path=/home/libcxx-builder/.buildkite-agent/builds/163fa794a6e7-1/llvm-project/libcxx-ci/build/generic-cxx03/projects/libcxx/test/std/iterators/iterator.primitives/iterator.traits/Output/empty.pass.cpp.dir/t.tmp/ModuleCache -Wno-macro-redefined -D_LIBCPP_HAS_THREAD_API_PTHREAD -Wno-macro-redefined -D_LIBCPP_ABI_VERSION=1 -lc++experimental -L/home/libcxx-builder/.buildkite-agent/builds/163fa794a6e7-1/llvm-project/libcxx-ci/build/generic-cxx03/./lib -Wl,-rpath,/home/libcxx-builder/.buildkite-agent/builds/163fa794a6e7-1/llvm-project/libcxx-ci/build/generic-cxx03/./lib -L/home/libcxx-builder/.buildkite-agent/builds/163fa794a6e7-1/llvm-project/libcxx-ci/build/generic-cxx03/./lib -Wl,-rpath,/home/libcxx-builder/.buildkite-agent/builds/163fa794a6e7-1/llvm…
260 mslibcxx CI C++11 > libc++.std/iterators/iterator_primitives/iterator_traits::empty.pass.cpp
Script: -- : 'COMPILED WITH'; /usr/bin/clang++ /home/libcxx-builder/.buildkite-agent/builds/c76d42bee082-1/llvm-project/libcxx-ci/libcxx/test/std/iterators/iterator.primitives/iterator.traits/empty.pass.cpp -v --target=x86_64-unknown-linux-gnu -include /home/libcxx-builder/.buildkite-agent/builds/c76d42bee082-1/llvm-project/libcxx-ci/libcxx/test/support/nasty_macros.h -nostdinc++ -I/home/libcxx-builder/.buildkite-agent/builds/c76d42bee082-1/llvm-project/libcxx-ci/build/generic-cxx11/include/c++/v1 -I/home/libcxx-builder/.buildkite-agent/builds/c76d42bee082-1/llvm-project/libcxx-ci/build/generic-cxx11/projects/libcxx/include/c++build -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -D__STDC_CONSTANT_MACROS -I/home/libcxx-builder/.buildkite-agent/builds/c76d42bee082-1/llvm-project/libcxx-ci/libcxx/test/support -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -std=c++11 -Werror -Wall -Wextra -Wshadow -Wundef -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-c++11-extensions -Wno-user-defined-literals -Wno-noexcept-type -Wno-atomic-alignment -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -D_LIBCPP_DISABLE_AVAILABILITY -fcoroutines-ts -Werror=thread-safety -Wuser-defined-warnings -fmodules-cache-path=/home/libcxx-builder/.buildkite-agent/builds/c76d42bee082-1/llvm-project/libcxx-ci/build/generic-cxx11/projects/libcxx/test/std/iterators/iterator.primitives/iterator.traits/Output/empty.pass.cpp.dir/t.tmp/ModuleCache -Wno-macro-redefined -D_LIBCPP_HAS_THREAD_API_PTHREAD -Wno-macro-redefined -D_LIBCPP_ABI_VERSION=1 -lc++experimental -L/home/libcxx-builder/.buildkite-agent/builds/c76d42bee082-1/llvm-project/libcxx-ci/build/generic-cxx11/./lib -Wl,-rpath,/home/libcxx-builder/.buildkite-agent/builds/c76d42bee082-1/llvm-project/libcxx-ci/build/generic-cxx11/./lib -L/home/libcxx-builder/.buildkite-agent/builds/c76d42bee082-1/llvm-project/libcxx-ci/build/generic-cxx11/./lib -Wl,-rpath,/home/libcxx-builder/.buildkite-agent/builds/c76d42bee082-1/llvm…
260 mslibcxx CI C++14 > libc++.std/iterators/iterator_primitives/iterator_traits::empty.pass.cpp
Script: -- : 'COMPILED WITH'; /usr/bin/clang++ /home/libcxx-builder/.buildkite-agent/builds/163fa794a6e7-1/llvm-project/libcxx-ci/libcxx/test/std/iterators/iterator.primitives/iterator.traits/empty.pass.cpp -v --target=x86_64-unknown-linux-gnu -include /home/libcxx-builder/.buildkite-agent/builds/163fa794a6e7-1/llvm-project/libcxx-ci/libcxx/test/support/nasty_macros.h -nostdinc++ -I/home/libcxx-builder/.buildkite-agent/builds/163fa794a6e7-1/llvm-project/libcxx-ci/build/generic-cxx14/include/c++/v1 -I/home/libcxx-builder/.buildkite-agent/builds/163fa794a6e7-1/llvm-project/libcxx-ci/build/generic-cxx14/projects/libcxx/include/c++build -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -D__STDC_CONSTANT_MACROS -I/home/libcxx-builder/.buildkite-agent/builds/163fa794a6e7-1/llvm-project/libcxx-ci/libcxx/test/support -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -std=c++14 -Werror -Wall -Wextra -Wshadow -Wundef -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-c++11-extensions -Wno-user-defined-literals -Wno-noexcept-type -Wno-atomic-alignment -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -D_LIBCPP_DISABLE_AVAILABILITY -fcoroutines-ts -Werror=thread-safety -Wuser-defined-warnings -fmodules-cache-path=/home/libcxx-builder/.buildkite-agent/builds/163fa794a6e7-1/llvm-project/libcxx-ci/build/generic-cxx14/projects/libcxx/test/std/iterators/iterator.primitives/iterator.traits/Output/empty.pass.cpp.dir/t.tmp/ModuleCache -Wno-macro-redefined -D_LIBCPP_HAS_THREAD_API_PTHREAD -Wno-macro-redefined -D_LIBCPP_ABI_VERSION=1 -lc++experimental -L/home/libcxx-builder/.buildkite-agent/builds/163fa794a6e7-1/llvm-project/libcxx-ci/build/generic-cxx14/./lib -Wl,-rpath,/home/libcxx-builder/.buildkite-agent/builds/163fa794a6e7-1/llvm-project/libcxx-ci/build/generic-cxx14/./lib -L/home/libcxx-builder/.buildkite-agent/builds/163fa794a6e7-1/llvm-project/libcxx-ci/build/generic-cxx14/./lib -Wl,-rpath,/home/libcxx-builder/.buildkite-agent/builds/163fa794a6e7-1/llvm…
280 mslibcxx CI C++17 > libc++.std/iterators/iterator_primitives/iterator_traits::empty.pass.cpp
Script: -- : 'COMPILED WITH'; /usr/bin/clang++ /home/libcxx-builder/.buildkite-agent/builds/7b5f5ea98a39-1/llvm-project/libcxx-ci/libcxx/test/std/iterators/iterator.primitives/iterator.traits/empty.pass.cpp -v --target=x86_64-unknown-linux-gnu -include /home/libcxx-builder/.buildkite-agent/builds/7b5f5ea98a39-1/llvm-project/libcxx-ci/libcxx/test/support/nasty_macros.h -nostdinc++ -I/home/libcxx-builder/.buildkite-agent/builds/7b5f5ea98a39-1/llvm-project/libcxx-ci/build/generic-cxx17/include/c++/v1 -I/home/libcxx-builder/.buildkite-agent/builds/7b5f5ea98a39-1/llvm-project/libcxx-ci/build/generic-cxx17/projects/libcxx/include/c++build -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -D__STDC_CONSTANT_MACROS -I/home/libcxx-builder/.buildkite-agent/builds/7b5f5ea98a39-1/llvm-project/libcxx-ci/libcxx/test/support -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -std=c++17 -Werror -Wall -Wextra -Wshadow -Wundef -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-c++11-extensions -Wno-user-defined-literals -Wno-noexcept-type -Wno-atomic-alignment -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -D_LIBCPP_DISABLE_AVAILABILITY -fcoroutines-ts -Werror=thread-safety -Wuser-defined-warnings -fmodules-cache-path=/home/libcxx-builder/.buildkite-agent/builds/7b5f5ea98a39-1/llvm-project/libcxx-ci/build/generic-cxx17/projects/libcxx/test/std/iterators/iterator.primitives/iterator.traits/Output/empty.pass.cpp.dir/t.tmp/ModuleCache -Wno-macro-redefined -D_LIBCPP_HAS_THREAD_API_PTHREAD -Wno-macro-redefined -D_LIBCPP_ABI_VERSION=1 -lc++experimental -L/home/libcxx-builder/.buildkite-agent/builds/7b5f5ea98a39-1/llvm-project/libcxx-ci/build/generic-cxx17/./lib -Wl,-rpath,/home/libcxx-builder/.buildkite-agent/builds/7b5f5ea98a39-1/llvm-project/libcxx-ci/build/generic-cxx17/./lib -L/home/libcxx-builder/.buildkite-agent/builds/7b5f5ea98a39-1/llvm-project/libcxx-ci/build/generic-cxx17/./lib -Wl,-rpath,/home/libcxx-builder/.buildkite-agent/builds/7b5f5ea98a39-1/llvm…
2,440 mslibcxx CI No locale > libc++.std/iterators/iterator_primitives/iterator_traits::cxx20_iterator_traits.compile.pass.cpp
Script: -- : 'COMPILED WITH'; /usr/bin/clang++ /home/libcxx-builder/.buildkite-agent/builds/e2c27bb48aa0-1/llvm-project/libcxx-ci/libcxx/test/std/iterators/iterator.primitives/iterator.traits/cxx20_iterator_traits.compile.pass.cpp -v --target=x86_64-unknown-linux-gnu -include /home/libcxx-builder/.buildkite-agent/builds/e2c27bb48aa0-1/llvm-project/libcxx-ci/libcxx/test/support/nasty_macros.h -nostdinc++ -I/home/libcxx-builder/.buildkite-agent/builds/e2c27bb48aa0-1/llvm-project/libcxx-ci/build/generic-no-localization/include/c++/v1 -I/home/libcxx-builder/.buildkite-agent/builds/e2c27bb48aa0-1/llvm-project/libcxx-ci/build/generic-no-localization/projects/libcxx/include/c++build -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -D__STDC_CONSTANT_MACROS -I/home/libcxx-builder/.buildkite-agent/builds/e2c27bb48aa0-1/llvm-project/libcxx-ci/libcxx/test/support -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -std=c++20 -Werror -Wall -Wextra -Wshadow -Wundef -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-c++11-extensions -Wno-user-defined-literals -Wno-noexcept-type -Wno-atomic-alignment -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -D_LIBCPP_DISABLE_AVAILABILITY -fcoroutines-ts -Werror=thread-safety -Wuser-defined-warnings -fmodules-cache-path=/home/libcxx-builder/.buildkite-agent/builds/e2c27bb48aa0-1/llvm-project/libcxx-ci/build/generic-no-localization/projects/libcxx/test/std/iterators/iterator.primitives/iterator.traits/Output/cxx20_iterator_traits.compile.pass.cpp.dir/t.tmp/ModuleCache -Wno-macro-redefined -D_LIBCPP_HAS_THREAD_API_PTHREAD -Wno-macro-redefined -D_LIBCPP_ABI_VERSION=1 -Wno-macro-redefined -D_LIBCPP_HAS_NO_LOCALIZATION -fsyntax-only
View Full Test Results (6 Failed)

Event Timeline

zoecarver created this revision.Apr 16 2021, 2:21 PM
zoecarver requested review of this revision.Apr 16 2021, 2:21 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 16 2021, 2:21 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript

I like these cleanups!

libcxx/include/__iterator/iterator_traits.h
15

Are we expecting <concepts> to include <type_traits>?

libcxx/include/iterator
416

The new header includes <concepts>. What's our policy for including this header here too? I would prefer to remove it.

Quuxplusone added inline comments.Apr 17 2021, 8:26 AM
libcxx/include/__iterator/iterator_traits.h
15

Yes. Everything includes <type_traits>; that's kind of an invariant of libc++.
However, if this file needs something out of <type_traits>, it's good practice to Include What You Use, preferably with a comment. E.g.

#include <type_traits> // declval
libcxx/include/iterator
416

Include What You Use. If this file uses something out of <concepts>, it should include <concepts>, preferably with a comment. However, in this case, the inclusion is actually mandated by the Standard! so we should probably comment to that effect.

#include <concepts> // mandated
cjdb accepted this revision.Apr 17 2021, 5:16 PM
Mordante accepted this revision as: Mordante.Apr 18 2021, 2:16 AM

LGTM assuming fixing the CI only requires include fixes.

libcxx/include/__iterator/iterator_traits.h
15

Fair point, but I disagree with the comment part. Unless it's a real surprise I dislike these comments since they tend to get outdated quickly.
A good example is <array> it includes <cstdlib> to use _LIBCPP_UNREACHABLE. It seems that macro is in that specific header since it may result in abort() which is in <cstdlib>.

ldionne accepted this revision.Apr 19 2021, 7:27 AM

LGTM but:

  1. Sort includes
  2. Make sure you include-what-you-use

Agreed with @Mordante, only add a comment explaining why you add an #include when it's not obvious why the #include is needed, otherwise those comments become outdated too quickly to be useful.

libcxx/include/iterator
419–422

Please sort those includes.

This revision is now accepted and ready to land.Apr 19 2021, 7:27 AM
zoecarver updated this revision to Diff 338647.Apr 19 2021, 2:43 PM
  • Fix includes.
  • Rebase off D99855.

Assuming the tests pass here as well, I'm going to land this with D99855.

This revision was landed with ongoing or failed builds.Apr 20 2021, 8:32 AM
This revision was automatically updated to reflect the committed changes.