<div dir="ltr">Committed as r285152.<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Oct 25, 2016 at 3:09 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>Missed one change from the test suite:</div><div><br></div><div>Index: test/Modules/cstd.m</div><div>==============================<wbr>==============================<wbr>=======</div><div>--- test/Modules/cstd.m (revision 285117)</div><div>+++ test/Modules/cstd.m (working copy)</div><div>@@ -1,5 +1,5 @@</div><div> // RUN: rm -rf %t</div><div>-// RUN: %clang_cc1 -fsyntax-only -isystem %S/Inputs/System/usr/include -ffreestanding -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -D__need_wint_t -Werror=implicit-function-<wbr>declaration %s</div><div>+// RUN: %clang_cc1 -fsyntax-only -internal-isystem %S/Inputs/System/usr/include -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -D__need_wint_t -Werror=implicit-function-<wbr>declaration %s</div><div> </div><div> @import uses_other_constants;</div><div> const double other_value = DBL_MAX;</div><div><br></div></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Oct 25, 2016 at 2:56 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">This was a thinko on my part: clang's builtin headers include_next the system headers, not the other way around, so the system headers should be implicitly textual, not clang's headers. This patch fixes the problem for me with glibc. Does this help for Darwin too?</div><div class="m_1392822330412458553HOEnZb"><div class="m_1392822330412458553h5"><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Oct 25, 2016 at 2:01 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div class="m_1392822330412458553m_483018273895972968h5">On Mon, Oct 24, 2016 at 4:58 PM, Bruno Cardoso Lopes via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>On Mon, Oct 24, 2016 at 4:17 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>> wrote:<br>
> On Mon, Oct 24, 2016 at 3:30 PM, Bruno Cardoso Lopes<br>
> <<a href="mailto:bruno.cardoso@gmail.com" target="_blank">bruno.cardoso@gmail.com</a>> wrote:<br>
>><br>
>> > Sure, go ahead.<br>
>><br>
>> I committed in r284797 and r284801 (libcxx). There's one minor issue<br>
>> I've found: the changes for the builtins affecting non submodule local<br>
>> visibility broke current users of plain "-fmodules" against our<br>
>> frameworks in public SDKs, in 10.11 & 10.12. I've attached a patch to<br>
>> work around that for the time being: make the new behavior dependent<br>
>> on local vis. Can you take a look?<br>
><br>
><br>
> What's the nature of the breakage? Generally I'd be fine with your patch,<br>
> but I wonder if there's something better we could do here.<br>
<br>
</span>I haven't entirely isolated the problem, but they are all related to<br>
definitions from stdint.h. In one example below, uint32_t doesn't<br>
leak, requiring an explicit "#include <stdint.h>" to make it work.<br>
<br>
-- example.m<br>
#import <IOKit/IODataQueueClient.h><br>
--<br>
$ clang -arch x86_64 -isysroot<br>
/Applications/Xcode.app/Conten<wbr>ts/Developer/Platforms/MacOSX.<wbr>platform/Developer/SDKs/MacOSX<wbr>10.12.sdk<br>
-fmodules-cache-path=tmpcache example.m -E -o /dev/null -fmodules<br>
<br>
While building module 'IOKit' imported from example.m:1:<br>
In file included from <module-includes>:2:<br>
/Applications/Xcode.app/Conten<wbr>ts/Developer/Platforms/MacOSX.<wbr>platform/Developer/SDKs/MacOSX<wbr>10.12.sdk/System/Library/Frame<wbr>works/IOKit.framework/Headers/<wbr>IODataQueueClient.h:62:71:<br>
error: de<br>
'Darwin.POSIX._types._uint32_t<wbr>' before it is required<br>
IOReturn IODataQueueDequeue(IODataQueue<wbr>Memory *dataQueue, void *data,<br>
uint32_t *dataSize);<br>
^<br>
/Applications/Xcode.app/Conten<wbr>ts/Developer/Platforms/MacOSX.<wbr>platform/Developer/SDKs/MacOSX<wbr>10.12.sdk/usr/include/_types/_<wbr>uint32_t.h:31:22:<br>
<span>note: previous declaration is here<br>
</span>typedef unsigned int uint32_t;<br>
^<br>
bot.m:1:9: fatal error: could not build module 'IOKit'<br>
#import <IOKit/IODataQueueClient.h><br>
~~~~~~~^</blockquote><div><br></div></div></div><div>This change also broke local submodule visibility builds with modular glibc (see PR30778 for details). I have an idea for how to fix this; running it through bootstrap now.</div><span><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>
>> > Hmm. Ideally, we should try to pick something that captures the spirit<br>
>> > of<br>
>> > "only non-modular headers and headers from used modules". Something like<br>
>> > "ignore_modules_not_declared_u<wbr>sed", but less wordy?<br>
>><br>
>> Right. It's gonna be hard to shrink this to a meaningful short name.<br>
>> What about a more generic "no_escape"? "no_undeclared_headers"?<br>
><br>
><br>
> Hmm. Maybe we could allow the existing [exhaustive] attribute to be<br>
> specified on a use-declaration:<br>
><br>
> use [exhaustive] a, b, c<br>
<br>
</span>I don't understand, the 'Darwin' module map doesn't use the 'use'<br>
keyword in any of its modules, how do you suggest we would use that to<br>
express the 'ignore_modules_not_declared_u<wbr>sed' idea?</blockquote><div><br></div></span><div>Hah, right, this would only work if your module has dependencies. Maybe an [exhaustive_uses] attribute on the module itself then? </div></div></div></div>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>