r310605 - [Modules] Prevent #import to reenter header if not building a module.

[Richard Smith via cfe-commits]

On 10 August 2017 at 10:42, Hans Wennborg via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> Sounds good to me.
>
> Richard, what do you think?
>
> On Thu, Aug 10, 2017 at 9:38 AM, Bruno Cardoso Lopes
> <bruno.cardoso at gmail.com> wrote:
> > Hi Hans, can we please get this merged into 5.0?
> >
> > Thanks,
> >
> > On Thu, Aug 10, 2017 at 12:16 PM, Bruno Cardoso Lopes via cfe-commits
> > <cfe-commits at lists.llvm.org> wrote:
> >> Author: bruno
> >> Date: Thu Aug 10 08:16:24 2017
> >> New Revision: 310605
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=310605&view=rev
> >> Log:
> >> [Modules] Prevent #import to reenter header if not building a module.
> >>
> >> When non-modular headers are imported while not building a module but
> >> in -fmodules mode, be conservative and preserve the default #import
> >> semantic: do not reenter headers.
>

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?".)

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.

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.

>>
> >> rdar://problem/33745031
> >>
> >> Added:
> >>     cfe/trunk/test/Modules/Inputs/import-textual/x.h
> >>     cfe/trunk/test/Modules/import-textual-nomodules.m
> >> Modified:
> >>     cfe/trunk/lib/Lex/HeaderSearch.cpp
> >>
> >> Modified: cfe/trunk/lib/Lex/HeaderSearch.cpp
> >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/
> HeaderSearch.cpp?rev=310605&r1=310604&r2=310605&view=diff
> >> ============================================================
> ==================
> >> --- cfe/trunk/lib/Lex/HeaderSearch.cpp (original)
> >> +++ cfe/trunk/lib/Lex/HeaderSearch.cpp Thu Aug 10 08:16:24 2017
> >> @@ -1143,7 +1143,7 @@ bool HeaderSearch::ShouldEnterIncludeFil
> >>      // headers find in the wild might rely only on #import and do not
> contain
> >>      // controlling macros, be conservative and only try to enter
> textual headers
> >>      // if such macro is present.
> >> -    if (!FileInfo.isModuleHeader &&
> >> +    if (FileInfo.isCompilingModuleHeader && !FileInfo.isModuleHeader
> &&
> >>          FileInfo.getControllingMacro(ExternalLookup))
> >>        TryEnterHdr = true;
> >>      return TryEnterHdr;
> >>
> >> Added: cfe/trunk/test/Modules/Inputs/import-textual/x.h
> >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/
> Modules/Inputs/import-textual/x.h?rev=310605&view=auto
> >> ============================================================
> ==================
> >> --- cfe/trunk/test/Modules/Inputs/import-textual/x.h (added)
> >> +++ cfe/trunk/test/Modules/Inputs/import-textual/x.h Thu Aug 10
> 08:16:24 2017
> >> @@ -0,0 +1,6 @@
> >> +#ifndef RANDOM_DEP
> >> +
> >> + at interface X
> >> + at end
> >> +
> >> +#endif // RANDOM_DEP
> >>
> >> Added: cfe/trunk/test/Modules/import-textual-nomodules.m
> >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/
> Modules/import-textual-nomodules.m?rev=310605&view=auto
> >> ============================================================
> ==================
> >> --- cfe/trunk/test/Modules/import-textual-nomodules.m (added)
> >> +++ cfe/trunk/test/Modules/import-textual-nomodules.m Thu Aug 10
> 08:16:24 2017
> >> @@ -0,0 +1,8 @@
> >> +// RUN: rm -rf %t
> >> +// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps
> -I%S/Inputs/import-textual -fmodules-cache-path=%t %s -verify
> >> +
> >> +// expected-no-diagnostics
> >> +
> >> +#import "x.h"
> >> +#import "x.h"
> >> +
> >>
> >>
> >> _______________________________________________
> >> cfe-commits mailing list
> >> cfe-commits at lists.llvm.org
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> >
> >
> >
> > --
> > Bruno Cardoso Lopes
> > http://www.brunocardoso.cc
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170810/23eaa645/attachment.html>