This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Update and normalize the "all the headers" tests.
ClosedPublic

Authored by Quuxplusone on Dec 2 2020, 4:40 PM.

Details

Summary

As I promised @ldionne in D92240! :) Some C++20 headers weren't added properly to all three of these test files. Add them, and take the time to normalize the formatting so that

diff <(grep '#include' foo.cpp) <(grep '#include' bar.cpp)

shows no diffs (except that no_assert_include deliberately excludes <cassert>).

Diff Detail

Event Timeline

Quuxplusone created this revision.Dec 2 2020, 4:40 PM
Herald added a reviewer: Restricted Project. · View Herald TranscriptDec 2 2020, 4:40 PM
Quuxplusone requested review of this revision.Dec 2 2020, 4:40 PM

Nice findings! Apparently all of barrier, latch, semaphore and simd are missing _LIBCPP_PUSH_MACROS #include <__undef_macros>. And semaphore includes <cassert>.

libcxx/test/libcxx/no_assert_include.compile.pass.cpp
100

You found the culprit!

  • Remove an unused include of <cassert> from <semaphore>.
  • Add macro guards to <{barrier,latch,semaphore}>.
  • Add macro guards to <experimental/simd>.
ldionne accepted this revision.Dec 3 2020, 7:45 AM

Thank you! I think it would be neat to add a script that can generate those, or generate them from CMake. Anything that would remove the need to do it manually would be great. WDYT?

This revision is now accepted and ready to land.Dec 3 2020, 7:45 AM

Two semaphore tests were relying on <semaphore> including <cassert>. Include-what-you-use in the semaphore tests.

I think it would be neat to add a script that can generate those, or generate them from CMake. Anything that would remove the need to do it manually would be great. WDYT?

I'm OCD enough not to mind doing it by hand, but I think engineering-wise your position is more defensible than mine. ;)
We already have libcxx/utils/generate_feature_test_macro_components.py; presumably we could generate these similarly. I'm not volunteering to write the Python.

ldionne accepted this revision.Dec 3 2020, 8:54 AM

Thanks! I'll look into automating that. I've been thinking about it for a while, and it has caused issues internally in the past so there's justification for me to look into it.