<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On 10 August 2017 at 10:42, Hans Wennborg 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></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Sounds good to me.<br>
<br>
Richard, what do you think?<br>
<div class="HOEnZb"><div class="h5"><br>
On Thu, Aug 10, 2017 at 9:38 AM, Bruno Cardoso Lopes<br>
<<a href="mailto:bruno.cardoso@gmail.com">bruno.cardoso@gmail.com</a>> wrote:<br>
> Hi Hans, can we please get this merged into 5.0?<br>
><br>
> Thanks,<br>
><br>
> On Thu, Aug 10, 2017 at 12:16 PM, Bruno Cardoso Lopes via cfe-commits<br>
> <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br>
>> Author: bruno<br>
>> Date: Thu Aug 10 08:16:24 2017<br>
>> New Revision: 310605<br>
>><br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=310605&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=310605&view=rev</a><br>
>> Log:<br>
>> [Modules] Prevent #import to reenter header if not building a module.<br>
>><br>
>> When non-modular headers are imported while not building a module but<br>
>> in -fmodules mode, be conservative and preserve the default #import<br>
>> semantic: do not reenter headers.<br></div></div></blockquote><div><br></div><div>This comment doesn't appear to describe what this patch does: even when building a module it does not re-enter headers except for ones declared 'textual header' within the module being built. (isCompilingModuleHeader doesn't mean "are we compiling a module header right now?", it means "is this file declared as some kind of header within the current CompilingModule?".)</div><div><br></div><div>I'm nervous about taking this change: it will presumably break some cases (particularly with local submodule visibility enabled) where a #pragma once header is #included into multiple headers in the same module, by refusing to re-enter such headers -- and it will fix other such cases, depending on what exactly the header does and the structure of the module.</div><div><br></div><div>If this were restricted to the case where we're not building a module (!LangOpts.isCompilingModule()), then I'd be OK putting it on the branch.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">
>><br>
>> rdar://problem/33745031<br>
>><br>
>> Added:<br>
>> cfe/trunk/test/Modules/Inputs/<wbr>import-textual/x.h<br>
>> cfe/trunk/test/Modules/import-<wbr>textual-nomodules.m<br>
>> Modified:<br>
>> cfe/trunk/lib/Lex/<wbr>HeaderSearch.cpp<br>
>><br>
>> Modified: cfe/trunk/lib/Lex/<wbr>HeaderSearch.cpp<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/HeaderSearch.cpp?rev=310605&r1=310604&r2=310605&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/lib/Lex/<wbr>HeaderSearch.cpp?rev=310605&<wbr>r1=310604&r2=310605&view=diff</a><br>
>> ==============================<wbr>==============================<wbr>==================<br>
>> --- cfe/trunk/lib/Lex/<wbr>HeaderSearch.cpp (original)<br>
>> +++ cfe/trunk/lib/Lex/<wbr>HeaderSearch.cpp Thu Aug 10 08:16:24 2017<br>
>> @@ -1143,7 +1143,7 @@ bool HeaderSearch::<wbr>ShouldEnterIncludeFil<br>
>> // headers find in the wild might rely only on #import and do not contain<br>
>> // controlling macros, be conservative and only try to enter textual headers<br>
>> // if such macro is present.<br>
>> - if (!FileInfo.isModuleHeader &&<br>
>> + if (FileInfo.<wbr>isCompilingModuleHeader && !FileInfo.isModuleHeader &&<br>
>> FileInfo.getControllingMacro(<wbr>ExternalLookup))<br>
>> TryEnterHdr = true;<br>
>> return TryEnterHdr;<br>
>><br>
>> Added: cfe/trunk/test/Modules/Inputs/<wbr>import-textual/x.h<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/import-textual/x.h?rev=310605&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/test/<wbr>Modules/Inputs/import-textual/<wbr>x.h?rev=310605&view=auto</a><br>
>> ==============================<wbr>==============================<wbr>==================<br>
>> --- cfe/trunk/test/Modules/Inputs/<wbr>import-textual/x.h (added)<br>
>> +++ cfe/trunk/test/Modules/Inputs/<wbr>import-textual/x.h Thu Aug 10 08:16:24 2017<br>
>> @@ -0,0 +1,6 @@<br>
>> +#ifndef RANDOM_DEP<br>
>> +<br>
>> +@interface X<br>
>> +@end<br>
>> +<br>
>> +#endif // RANDOM_DEP<br>
>><br>
>> Added: cfe/trunk/test/Modules/import-<wbr>textual-nomodules.m<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/import-textual-nomodules.m?rev=310605&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/test/<wbr>Modules/import-textual-<wbr>nomodules.m?rev=310605&view=<wbr>auto</a><br>
>> ==============================<wbr>==============================<wbr>==================<br>
>> --- cfe/trunk/test/Modules/import-<wbr>textual-nomodules.m (added)<br>
>> +++ cfe/trunk/test/Modules/import-<wbr>textual-nomodules.m Thu Aug 10 08:16:24 2017<br>
>> @@ -0,0 +1,8 @@<br>
>> +// RUN: rm -rf %t<br>
>> +// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -I%S/Inputs/import-textual -fmodules-cache-path=%t %s -verify<br>
>> +<br>
>> +// expected-no-diagnostics<br>
>> +<br>
>> +#import "x.h"<br>
>> +#import "x.h"<br>
>> +<br>
>><br>
>><br>
>> ______________________________<wbr>_________________<br>
>> cfe-commits mailing list<br>
>> <a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a><br>
>> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br>
><br>
><br>
><br>
> --<br>
> Bruno Cardoso Lopes<br>
> <a href="http://www.brunocardoso.cc" rel="noreferrer" target="_blank">http://www.brunocardoso.cc</a><br>
______________________________<wbr>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br>
</div></div></blockquote></div><br></div></div>