Regression caused by r276159 - [modules] Don't emit initializers for VarDecls within a module eagerly whenever
Manman via cfe-commits
cfe-commits at lists.llvm.org
Mon Dec 19 17:23:21 PST 2016
> On Dec 14, 2016, at 1:20 PM, Manman <mren at apple.com> wrote:
>
>
>> On Jul 20, 2016, at 12:10 PM, Richard Smith via cfe-commits <cfe-commits at lists.llvm.org> wrote:
>>
>> Author: rsmith
>> Date: Wed Jul 20 14:10:16 2016
>> New Revision: 276159
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=276159&view=rev
>> Log:
>> [modules] Don't emit initializers for VarDecls within a module eagerly whenever
>> we first touch any part of that module. Instead, defer them until the first
>> time that module is (transitively) imported. The initializer step for a module
>> then recursively initializes modules that its own headers imported.
>>
>> For example, this avoids running the <iostream> global initializer in programs
>> that don't actually use iostreams, but do use other parts of the standard
>> library.
>>
>> Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=276159&r1=276158&r2=276159&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
>> +++ cfe/trunk/lib/Serialization/ASTWriter.cpp Wed Jul 20 14:10:16 2016
>> @@ -1017,6 +1017,7 @@ void ASTWriter::WriteBlockInfoBlock() {
>> RECORD(SUBMODULE_PRIVATE_HEADER);
>> RECORD(SUBMODULE_TEXTUAL_HEADER);
>> RECORD(SUBMODULE_PRIVATE_TEXTUAL_HEADER);
>> + RECORD(SUBMODULE_INITIALIZERS);
>>
>> // Comments Block.
>> BLOCK(COMMENTS_BLOCK);
>> @@ -2417,7 +2418,9 @@ unsigned ASTWriter::getLocalOrImportedSu
>> if (Known != SubmoduleIDs.end())
>> return Known->second;
>>
>> - if (Mod->getTopLevelModule() != WritingModule)
>> + auto *Top = Mod->getTopLevelModule();
>> + if (Top != WritingModule &&
>> + !Top->fullModuleNameIs(StringRef(getLangOpts().CurrentModule)))
>> return 0;
>>
>> return SubmoduleIDs[Mod] = NextSubmoduleID++;
>> @@ -2649,6 +2652,13 @@ void ASTWriter::WriteSubmodules(Module *
>> Stream.EmitRecordWithBlob(ConfigMacroAbbrev, Record, CM);
>> }
>>
>> + // Emit the initializers, if any.
>> + RecordData Inits;
>> + for (Decl *D : Context->getModuleInitializers(Mod))
>> + Inits.push_back(GetDeclRef(D));
>> + if (!Inits.empty())
>> + Stream.EmitRecord(SUBMODULE_INITIALIZERS, Inits);
>> +
>> // Queue up the submodules of this module.
>> for (auto *M : Mod->submodules())
>> Q.push(M);
>> @@ -4514,6 +4524,17 @@ uint64_t ASTWriter::WriteASTCore(Sema &S
>> // If we're emitting a module, write out the submodule information.
>> if (WritingModule)
>> WriteSubmodules(WritingModule);
>> + else if (!getLangOpts().CurrentModule.empty()) {
>> + // If we're building a PCH in the implementation of a module, we may need
>> + // the description of the current module.
>
> Hi Richard,
>
> Sorry to dig up this old commit. We are seeing a regression with the following simplified testing case.
> cat test.h
> #include "A.h"
> cat test.m
> #include "C.h"
> cat run.sh
> rm -rf tmp
> clang -cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=tmp -I Inputs -emit-pch -o tmp-A.pch test.h -fmodule-name=Contacts -x objective-c-header
> clang -cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=tmp -I Inputs -include-pch tmp-A.pch test.m -fsyntax-only -fmodule-name=Contacts
> test.m:1:2: fatal error: module 'Contacts' is defined in both '/Users/Ren/projects/module/screen/r29542516/tmp/2BSZ9VNPW89EZ/Contacts-389KKPMFZ1XO7.pcm' and 'tmp-A.pch'
> #include "C.h"
> ^
> test.m:1:2: note: imported by module 'CloudKit' in '/Users/Ren/projects/module/screen/r29542516/tmp/2BSZ9VNPW89EZ/CloudKit-389KKPMFZ1XO7.pcm'
> 1 error generated.
>
> cat Inputs/module.modulemap
> module CloudKit {
> header "C.h"
> export *
> }
>
> module Contacts {
> header "D.h"
> export *
> }
> cat Inputs/A.h
> // in pch
> cat Inputs/C.h
> #include "D.h"
> cat Inputs/D.h
> //empty
>
> We used to have a different option "-fmodule-implementation-of" which was unified with "-fmodule-name”.
>
> When building the pch, we pass in “-fmodule-implementation-of Contacts”, because of this part of the commit, we will say the pch defines module Contacts.
> The TU also pulls in module CloudKit, which imports module Contacts. Thus we have this error:
> module 'Contacts' is defined in both '/Users/Ren/projects/module/screen/r29542516/tmp/2BSZ9VNPW89EZ/Contacts-389KKPMFZ1XO7.pcm' and 'tmp-A.pch’
>
> I wonder why we want to say that a pch defines a module. Is this related to lazily evaluating initializers?
Hi Richard,
Can you comment on why we want to say that a PCH defines a module? Any suggestion on fixing this regression?
Manman
>
> Thanks,
> Manman
>
>> + //
>> + // FIXME: We may need other modules that we did not load from an AST file,
>> + // such as if a module declares a 'conflicts' on a different module.
>> + Module *M = PP.getHeaderSearchInfo().getModuleMap().findModule(
>> + getLangOpts().CurrentModule);
>> + if (M && !M->IsFromModuleFile)
>> + WriteSubmodules(M);
>> + }
>>
>> Stream.EmitRecord(SPECIAL_TYPES, SpecialTypes);
>>
>>
>
More information about the cfe-commits
mailing list