<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Jan 9, 2015 at 6:46 PM, Marshall Clow <span dir="ltr"><<a href="mailto:mclow.lists@gmail.com" target="_blank">mclow.lists@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><span class=""><br><div><blockquote type="cite"><div>On Jan 9, 2015, at 5:52 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>> wrote:</div><br><div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Jan 9, 2015 at 12:32 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span>On Fri, Jan 9, 2015 at 12:25 PM, Marshall Clow <span dir="ltr"><<a href="mailto:mclow.lists@gmail.com" target="_blank">mclow.lists@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Author: marshall<br>
Date: Fri Jan  9 14:25:52 2015<br>
New Revision: 225541<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=225541&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=225541&view=rev</a><br>
Log:<br>
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.<br></blockquote></span><div><br>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++)?<br></div></div></div></div></blockquote><div><br></div><div>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.</div></div></div></div></div></blockquote><br></div></span><div>Great!</div><div>Thanks.</div></div></blockquote><div><br></div><div>Yep, there's still /lots/ more issues. Looking through them, I broadly see three different categories:</div><div><br></div><div>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:</div><div><br></div><div>  <algorithm> uses numeric_limits but doesn't include <limits></div><div>  <algorithm> uses placement new but doesn't include <new></div><div><br></div><div>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.</div><div>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.</div><div><br></div><div>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.</div><div><br></div><div>Eventually, I'll fix Clang's visibility rules and we'll start detecting some of these cases when building the libc++ module too.</div><div><br></div><div>2) A test file uses a symbol that it didn't ask for. Here are some examples:</div><div><br></div><div>Files using std::pair without including <utility>:</div><div><div>std/algorithms/alg.modifying.operations/alg.partitions/partition_copy.pass.cpp</div></div><div>std/algorithms/alg.modifying.operations/alg.partitions/stable_partition.pass.cpp</div><div>std/algorithms/alg.nonmodifying/mismatch/mismatch_pred.pass.cpp<br></div><div>std/algorithms/alg.nonmodifying/mismatch/mismatch.pass.cpp<br></div><div>std/algorithms/alg.sorting/alg.binary.search/equal.range/equal_range_comp.pass.cpp<br></div><div>std/algorithms/alg.sorting/alg.binary.search/equal.range/equal_range.pass.cpp<br></div><div>std/algorithms/alg.sorting/alg.min.max/minmax_element_comp.pass.cpp</div><div>std/algorithms/alg.sorting/alg.min.max/minmax.pass.cpp<br></div><div>[... and so on ...]</div><div><br></div><div>test/support/min_allocator.h uses enable_if but doesn't include <type_traits><br></div><div><br></div><div>3) A symbol is defined in the wrong header. Examples:</div><div><br></div><div>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>.</div><div><br></div><div>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>.</div><div><br></div><div><br></div><div>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):</div><div><br></div><div>1) Modify libcxx/test/libcxx/test/config.py to add '-fmodules' to compile_flags.</div><div>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:</div><div>  ninja check-libcxx  2238.75s user 259.10s system 1913% cpu 2:10.51 total, without modules<br></div><div>  ninja check-libcxx  907.54s user 256.34s system 1103% cpu 1:45.50 total, with modules<br></div><div>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)</div><div>4) Run the tests again, weep quietly.</div></div></div></div>