[libcxx] r225541 - Walter Brown sent a list of tests which needed 'additional includes' to match what was in the standard. Added these includes to the tests. No changes to the library or test results.

[Richard Smith]

On Fri, Jan 9, 2015 at 6:46 PM, Marshall Clow <mclow.lists at gmail.com> wrote:

>
> On Jan 9, 2015, at 5:52 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>
> On Fri, Jan 9, 2015 at 12:32 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>> On Fri, Jan 9, 2015 at 12:25 PM, Marshall Clow <mclow.lists at gmail.com>
>> wrote:
>>
>>> Author: marshall
>>> Date: Fri Jan  9 14:25:52 2015
>>> New Revision: 225541
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=225541&view=rev
>>> Log:
>>> Walter Brown sent a list of tests which needed 'additional includes' to
>>> match what was in the standard. Added these includes to the tests. No
>>> changes to the library or test results.
>>>
>>
>> Richard - would a modularized libc++ (you have the setup for that, yes?)
>> help enforce the right header inclusions in these tests (& other code using
>> libc++)?
>>
>
> I have such a module map, but I've not run the libc++ tests with it in a
> long time. IIRC it found a *lot* of missing #includes, many of which were
> cases where <a> "obviously" includes <b> (but actually isn't required to do
> so). I'll have another look.
>
>
> Great!
> Thanks.
>

Yep, there's still /lots/ more issues. Looking through them, I broadly see
three different categories:

1) <foo> includes <bar>, <bar> happens to include <baz>, <foo> uses symbols
from <baz>. I guess you don't care about these? Due to the way we build
modules, this isn't detected while building the module, but when we
instantiate a template from <foo> that is supposed to find a symbol from
<baz>, that will fail. For instance:

  <algorithm> uses numeric_limits but doesn't include <limits>
  <algorithm> uses placement new but doesn't include <new>

In the first case, we use the template from within <algorithm> but then
later notice that the definition of it shouldn't be visible and complain.
In the second case, we defer overload resolution of the placement new until
the point of instantiation (because it has dependent arguments) and at that
point we notice that there are no visible, viable candidates for the call
to operator new.

In each case, certain template instantiations involving templates in
<algorithm> fail because neither the point of instantiation nor the point
of definition (nor anywhere else on the path of instantiation) can see the
symbols that are being used.

Eventually, I'll fix Clang's visibility rules and we'll start detecting
some of these cases when building the libc++ module too.

2) A test file uses a symbol that it didn't ask for. Here are some examples:

Files using std::pair without including <utility>:
std/algorithms/alg.modifying.operations/alg.partitions/partition_copy.pass.cpp
std/algorithms/alg.modifying.operations/alg.partitions/stable_partition.pass.cpp
std/algorithms/alg.nonmodifying/mismatch/mismatch_pred.pass.cpp
std/algorithms/alg.nonmodifying/mismatch/mismatch.pass.cpp
std/algorithms/alg.sorting/alg.binary.search/equal.range/equal_range_comp.pass.cpp
std/algorithms/alg.sorting/alg.binary.search/equal.range/equal_range.pass.cpp
std/algorithms/alg.sorting/alg.min.max/minmax_element_comp.pass.cpp
std/algorithms/alg.sorting/alg.min.max/minmax.pass.cpp
[... and so on ...]

test/support/min_allocator.h uses enable_if but doesn't include
<type_traits>

3) A symbol is defined in the wrong header. Examples:

std::iter_swap is defined in <type_traits> rather than <algorithm>. This
causes std/algorithms/alg.modifying.operations/alg.swap/iter_swap.pass.cpp
to fail because it doesn't include <type_traits>.

std::swap_ranges is defined in <utility> rather than <algorithm>. This
causes std/algorithms/alg.modifying.operations/alg.swap/swap_ranges.pass.cpp
to fail because it doesn't include <utility>.


If you want to try this yourself, it's quite straightforward (I'm assuming
you have an in-llvm-tree checkout and you're testing with lit):

1) Modify libcxx/test/libcxx/test/config.py to add '-fmodules' to
compile_flags.
2) At this point you can run the tests to see how they fare with modules
enabled. I get 7 test failures, which appear to be bugs with headers not
being sufficiently modular, especially wrt WCHAR_MIN and WCHAR_MAX macros.
And a nice runtime reduction:
  ninja check-libcxx  2238.75s user 259.10s system 1913% cpu 2:10.51 total,
without modules
  ninja check-libcxx  907.54s user 256.34s system 1103% cpu 1:45.50 total,
with modules
3) Remove all the "export *"s from libcxx/include/module.modulemap, and add
"export __functional_base" to the functional module. (There are probably
other re-exports of __headers we should add here, but that's the only one
I've hit so far)
4) Run the tests again, weep quietly.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150109/de9c97e3/attachment.html>