r291628 - Module: Do not create Implicit ImportDecl for module X if we

Manman Ren via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 10 19:14:32 PST 2017


On Tue, Jan 10, 2017 at 5:59 PM, Richard Smith <richard at metafoo.co.uk>
wrote:

> On 10 January 2017 at 17:57, Richard Smith <richard at metafoo.co.uk> wrote:
>
>> On 10 January 2017 at 16:48, Manman Ren via cfe-commits <
>> cfe-commits at lists.llvm.org> wrote:
>>
>>> Author: mren
>>> Date: Tue Jan 10 18:48:19 2017
>>> New Revision: 291628
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=291628&view=rev
>>> Log:
>>> Module: Do not create Implicit ImportDecl for module X if we
>>>  are building an implemenation of module X.
>>>
>>
>> Hmm. We do actually have an include mapping to a module header in this
>> case. Perhaps it would be better to create a faithful AST representation
>> and handle this in CodeGen instead -- we should not add any link flags when
>> an implementation TU of a module imports a header of that same module.
>>
>
> It's worth pointing out that the C++ modules TS allows a modular import of
> a module interface into an implementation unit of that module, so the
> current approach won't work for that case, but filtering these imports out
> in CodeGen would work.
>

Makes sense. I will commit a follow-up patch.

Manman

>
>
>> This fixes a regression caused by r280409.
>>> rdar://problem/29930553
>>>
>>> Added:
>>>     cfe/trunk/test/Modules/Inputs/module-impl-with-link/
>>>     cfe/trunk/test/Modules/Inputs/module-impl-with-link/foo.h
>>>     cfe/trunk/test/Modules/Inputs/module-impl-with-link/module.modulemap
>>>     cfe/trunk/test/Modules/module-impl-with-link.c
>>> Modified:
>>>     cfe/trunk/include/clang/Sema/Sema.h
>>>     cfe/trunk/lib/Sema/SemaDecl.cpp
>>>
>>> Modified: cfe/trunk/include/clang/Sema/Sema.h
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/
>>> Sema/Sema.h?rev=291628&r1=291627&r2=291628&view=diff
>>> ============================================================
>>> ==================
>>> --- cfe/trunk/include/clang/Sema/Sema.h (original)
>>> +++ cfe/trunk/include/clang/Sema/Sema.h Tue Jan 10 18:48:19 2017
>>> @@ -1905,7 +1905,8 @@ public:
>>>    /// \brief The parser has processed a module import translated from a
>>>    /// #include or similar preprocessing directive.
>>>    void ActOnModuleInclude(SourceLocation DirectiveLoc, Module *Mod);
>>> -  void BuildModuleInclude(SourceLocation DirectiveLoc, Module *Mod);
>>> +  void BuildModuleInclude(SourceLocation DirectiveLoc, Module *Mod,
>>> +                          bool NoImport);
>>>
>>>    /// \brief The parsed has entered a submodule.
>>>    void ActOnModuleBegin(SourceLocation DirectiveLoc, Module *Mod);
>>>
>>> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaD
>>> ecl.cpp?rev=291628&r1=291627&r2=291628&view=diff
>>> ============================================================
>>> ==================
>>> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
>>> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Tue Jan 10 18:48:19 2017
>>> @@ -15652,10 +15652,11 @@ DeclResult Sema::ActOnModuleImport(Sourc
>>>
>>>  void Sema::ActOnModuleInclude(SourceLocation DirectiveLoc, Module
>>> *Mod) {
>>>    checkModuleImportContext(*this, Mod, DirectiveLoc, CurContext, true);
>>> -  BuildModuleInclude(DirectiveLoc, Mod);
>>> +  BuildModuleInclude(DirectiveLoc, Mod, false/*NoImport*/);
>>>  }
>>>
>>> -void Sema::BuildModuleInclude(SourceLocation DirectiveLoc, Module
>>> *Mod) {
>>> +void Sema::BuildModuleInclude(SourceLocation DirectiveLoc, Module *Mod,
>>> +                              bool NoImport) {
>>>    // Determine whether we're in the #include buffer for a module. The
>>> #includes
>>>    // in that buffer do not qualify as module imports; they're just an
>>>    // implementation detail of us building the module.
>>> @@ -15665,7 +15666,7 @@ void Sema::BuildModuleInclude(SourceLoca
>>>        TUKind == TU_Module &&
>>>        getSourceManager().isWrittenInMainFile(DirectiveLoc);
>>>
>>> -  bool ShouldAddImport = !IsInModuleIncludes;
>>> +  bool ShouldAddImport = !IsInModuleIncludes && !NoImport;
>>>
>>>    // If this module import was due to an inclusion directive, create an
>>>    // implicit import declaration to capture it in the AST.
>>> @@ -15713,7 +15714,11 @@ void Sema::ActOnModuleEnd(SourceLocation
>>>    assert(File != getSourceManager().getMainFileID() &&
>>>           "end of submodule in main source file");
>>>    SourceLocation DirectiveLoc = getSourceManager().getIncludeLoc(File);
>>> -  BuildModuleInclude(DirectiveLoc, Mod);
>>> +  // Do not create implicit ImportDecl if we are building the
>>> implementation
>>> +  // of a module.
>>> +  bool NoImport = Mod->getTopLevelModuleName() ==
>>> getLangOpts().CurrentModule &&
>>> +                  !getLangOpts().isCompilingModule();
>>> +  BuildModuleInclude(DirectiveLoc, Mod, NoImport);
>>>  }
>>>
>>>  void Sema::createImplicitModuleImportForErrorRecovery(SourceLocation
>>> Loc,
>>>
>>> Added: cfe/trunk/test/Modules/Inputs/module-impl-with-link/foo.h
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/I
>>> nputs/module-impl-with-link/foo.h?rev=291628&view=auto
>>> ============================================================
>>> ==================
>>> --- cfe/trunk/test/Modules/Inputs/module-impl-with-link/foo.h (added)
>>> +++ cfe/trunk/test/Modules/Inputs/module-impl-with-link/foo.h Tue Jan
>>> 10 18:48:19 2017
>>> @@ -0,0 +1 @@
>>> +//empty
>>>
>>> Added: cfe/trunk/test/Modules/Inputs/module-impl-with-link/module.m
>>> odulemap
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/I
>>> nputs/module-impl-with-link/module.modulemap?rev=291628&view=auto
>>> ============================================================
>>> ==================
>>> --- cfe/trunk/test/Modules/Inputs/module-impl-with-link/module.modulemap
>>> (added)
>>> +++ cfe/trunk/test/Modules/Inputs/module-impl-with-link/module.modulemap
>>> Tue Jan 10 18:48:19 2017
>>> @@ -0,0 +1,4 @@
>>> +module Clib {
>>> +  header "foo.h"
>>> +  link "Clib"
>>> +}
>>>
>>> Added: cfe/trunk/test/Modules/module-impl-with-link.c
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/m
>>> odule-impl-with-link.c?rev=291628&view=auto
>>> ============================================================
>>> ==================
>>> --- cfe/trunk/test/Modules/module-impl-with-link.c (added)
>>> +++ cfe/trunk/test/Modules/module-impl-with-link.c Tue Jan 10 18:48:19
>>> 2017
>>> @@ -0,0 +1,7 @@
>>> +// RUN: rm -rf %t
>>> +// RUN: %clang_cc1 -fmodules-cache-path=%t -fmodules
>>> -fimplicit-module-maps -fmodule-name=Clib %s -I
>>> %S/Inputs/module-impl-with-link -emit-llvm -o -
>>> +#include "foo.h"
>>> +// CHECK: !{{[0-9]+}} = !{i32 6, !"Linker Options",
>>> ![[LINK_OPTIONS:[0-9]+]]}
>>> +// Make sure we don't generate linker option for module Clib since this
>>> TU is
>>> +// an implementation of Clib.
>>> +// CHECK: ![[LINK_OPTIONS]] = !{}
>>>
>>>
>>> _______________________________________________
>>> 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/20170110/001b638b/attachment.html>


More information about the cfe-commits mailing list