Index: include/clang/Basic/DiagnosticCommonKinds.td =================================================================== --- include/clang/Basic/DiagnosticCommonKinds.td +++ include/clang/Basic/DiagnosticCommonKinds.td @@ -94,6 +94,7 @@ "could not acquire lock file for module '%0': %1">, InGroup; def remark_module_lock_timeout : Remark< "timed out waiting to acquire lock file for module '%0'">, InGroup; +def err_module_shadowed : Error<"import of shadowed module '%0'">, DefaultFatal; def err_module_cycle : Error<"cyclic dependency in module '%0': %1">, DefaultFatal; def err_module_prebuilt : Error< Index: include/clang/Basic/Module.h =================================================================== --- include/clang/Basic/Module.h +++ include/clang/Basic/Module.h @@ -157,6 +157,9 @@ /// will be false to indicate that this (sub)module is not available. SmallVector Requirements; + /// \brief A module with the same name that shadows this module. + Module *ShadowingModule = nullptr; + /// \brief Whether this module is missing a feature from \c Requirements. unsigned IsMissingRequirement : 1; @@ -337,13 +340,20 @@ /// /// \param Target The target options used for the current translation unit. /// - /// \param Req If this module is unavailable, this parameter - /// will be set to one of the requirements that is not met for use of - /// this module. + /// \param Req If this module is unavailable because of a missing requirement, + /// this parameter will be set to one of the requirements that is not met for + /// use of this module. + /// + /// \param MissingHeader If this module is unavailable because of a missing + /// header, this parameter will be set to one of the missing headers. + /// + /// \param ShadowingModule If this module is unavailable because it is + /// shadowed, this parameter will be set to the shadowing module. bool isAvailable(const LangOptions &LangOpts, const TargetInfo &Target, Requirement &Req, - UnresolvedHeaderDirective &MissingHeader) const; + UnresolvedHeaderDirective &MissingHeader, + Module *&ShadowingModule) const; /// \brief Determine whether this module is a submodule. bool isSubModule() const { return Parent != nullptr; } Index: include/clang/Lex/HeaderSearch.h =================================================================== --- include/clang/Lex/HeaderSearch.h +++ include/clang/Lex/HeaderSearch.h @@ -683,7 +683,8 @@ LoadModuleMapResult loadModuleMapFileImpl(const FileEntry *File, bool IsSystem, - const DirectoryEntry *Dir); + const DirectoryEntry *Dir, + bool IsExplicitlyProvided); /// \brief Try to load the module map file in the given directory. /// Index: include/clang/Lex/ModuleMap.h =================================================================== --- include/clang/Lex/ModuleMap.h +++ include/clang/Lex/ModuleMap.h @@ -174,6 +174,18 @@ /// header. llvm::DenseMap UmbrellaDirs; + /// \brief The set of modules provided explicitly (e.g. by -fmodule-map-file), + /// which are allowed to shadow other implicitly discovered modules. + llvm::DenseSet ExplicitlyProvidedModules; + + bool CurrentModuleMapIsExplicitlyProvided = false; + + bool mayShadowModuleBeingParsed(Module *ExistingModule) { + assert(!ExistingModule->Parent && "expected top-level module"); + return !CurrentModuleMapIsExplicitlyProvided && + ExplicitlyProvidedModules.count(ExistingModule); + } + /// \brief The set of attributes that can be attached to a module. struct Attributes { Attributes() @@ -440,6 +452,11 @@ Module *inferFrameworkModule(const DirectoryEntry *FrameworkDir, bool IsSystem, Module *Parent); + /// \brief Create a new top-level module that is shadowed by + /// \p ShadowingModule. + Module *createShadowedModule(StringRef Name, bool IsFramework, + Module *ShadowingModule); + /// \brief Retrieve the module map file containing the definition of the given /// module. /// @@ -535,6 +552,8 @@ /// \brief Marks this header as being excluded from the given module. void excludeHeader(Module *Mod, Module::Header Header); + void setExplicitlyProvided(Module *Mod); + /// \brief Parse the given module map file, and record any modules we /// encounter. /// @@ -549,11 +568,16 @@ /// \param ExternModuleLoc The location of the "extern module" declaration /// that caused us to load this module map file, if any. /// + /// \param IsExplicitlyProvided Whether this module map file was provided + /// explicitly by the user (e.g. -fmodule-map-file), rather than found + /// implicitly. + /// /// \returns true if an error occurred, false otherwise. bool parseModuleMapFile(const FileEntry *File, bool IsSystem, const DirectoryEntry *HomeDir, - SourceLocation ExternModuleLoc = SourceLocation()); - + SourceLocation ExternModuleLoc = SourceLocation(), + bool IsExplicitlyProvided = false); + /// \brief Dump the contents of the module map, for debugging purposes. void dump(); Index: lib/Basic/Module.cpp =================================================================== --- lib/Basic/Module.cpp +++ lib/Basic/Module.cpp @@ -83,11 +83,16 @@ bool Module::isAvailable(const LangOptions &LangOpts, const TargetInfo &Target, Requirement &Req, - UnresolvedHeaderDirective &MissingHeader) const { + UnresolvedHeaderDirective &MissingHeader, + Module *&ShadowingModule) const { if (IsAvailable) return true; for (const Module *Current = this; Current; Current = Current->Parent) { + if (Current->ShadowingModule) { + ShadowingModule = Current->ShadowingModule; + return false; + } for (unsigned I = 0, N = Current->Requirements.size(); I != N; ++I) { if (hasFeature(Current->Requirements[I].first, LangOpts, Target) != Current->Requirements[I].second) { Index: lib/Frontend/CompilerInstance.cpp =================================================================== --- lib/Frontend/CompilerInstance.cpp +++ lib/Frontend/CompilerInstance.cpp @@ -1811,8 +1811,13 @@ // Check whether this module is available. clang::Module::Requirement Requirement; clang::Module::UnresolvedHeaderDirective MissingHeader; + clang::Module *ShadowingModule = nullptr; if (!Module->isAvailable(getLangOpts(), getTarget(), Requirement, - MissingHeader)) { + MissingHeader, ShadowingModule)) { + + assert(!ShadowingModule && + "lookup of module by name should never find shadowed module"); + if (MissingHeader.FileNameLoc.isValid()) { getDiagnostics().Report(MissingHeader.FileNameLoc, diag::err_module_header_missing) Index: lib/Frontend/FrontendActions.cpp =================================================================== --- lib/Frontend/FrontendActions.cpp +++ lib/Frontend/FrontendActions.cpp @@ -348,8 +348,13 @@ // Check whether we can build this module at all. clang::Module::Requirement Requirement; clang::Module::UnresolvedHeaderDirective MissingHeader; + clang::Module *ShadowingModule = nullptr; if (!Module->isAvailable(CI.getLangOpts(), CI.getTarget(), Requirement, - MissingHeader)) { + MissingHeader, ShadowingModule)) { + + assert(!ShadowingModule && + "lookup of module by name should never find shadowed module"); + if (MissingHeader.FileNameLoc.isValid()) { CI.getDiagnostics().Report(MissingHeader.FileNameLoc, diag::err_module_header_missing) Index: lib/Lex/HeaderSearch.cpp =================================================================== --- lib/Lex/HeaderSearch.cpp +++ lib/Lex/HeaderSearch.cpp @@ -1336,7 +1336,8 @@ } } - switch (loadModuleMapFileImpl(File, IsSystem, Dir)) { + switch (loadModuleMapFileImpl(File, IsSystem, Dir, + /*IsExplictlyProvided=*/true)) { case LMM_AlreadyLoaded: case LMM_NewlyLoaded: return false; @@ -1349,7 +1350,8 @@ HeaderSearch::LoadModuleMapResult HeaderSearch::loadModuleMapFileImpl(const FileEntry *File, bool IsSystem, - const DirectoryEntry *Dir) { + const DirectoryEntry *Dir, + bool IsExplicitlyProvided) { assert(File && "expected FileEntry"); // Check whether we've already loaded this module map, and mark it as being @@ -1358,14 +1360,16 @@ if (!AddResult.second) return AddResult.first->second ? LMM_AlreadyLoaded : LMM_InvalidModuleMap; - if (ModMap.parseModuleMapFile(File, IsSystem, Dir)) { + if (ModMap.parseModuleMapFile(File, IsSystem, Dir, SourceLocation(), + IsExplicitlyProvided)) { LoadedModuleMaps[File] = false; return LMM_InvalidModuleMap; } // Try to load a corresponding private module map. if (const FileEntry *PMMFile = getPrivateModuleMap(File, FileMgr)) { - if (ModMap.parseModuleMapFile(PMMFile, IsSystem, Dir)) { + if (ModMap.parseModuleMapFile(PMMFile, IsSystem, Dir, SourceLocation(), + IsExplicitlyProvided)) { LoadedModuleMaps[File] = false; return LMM_InvalidModuleMap; } @@ -1437,8 +1441,8 @@ return KnownDir->second ? LMM_AlreadyLoaded : LMM_InvalidModuleMap; if (const FileEntry *ModuleMapFile = lookupModuleMapFile(Dir, IsFramework)) { - LoadModuleMapResult Result = - loadModuleMapFileImpl(ModuleMapFile, IsSystem, Dir); + LoadModuleMapResult Result = loadModuleMapFileImpl( + ModuleMapFile, IsSystem, Dir, /*IsExplicitlyProvided=*/false); // Add Dir explicitly in case ModuleMapFile is in a subdirectory. // E.g. Foo.framework/Modules/module.modulemap // ^Dir ^ModuleMapFile Index: lib/Lex/ModuleMap.cpp =================================================================== --- lib/Lex/ModuleMap.cpp +++ lib/Lex/ModuleMap.cpp @@ -29,6 +29,7 @@ #include "llvm/Support/FileSystem.h" #include "llvm/Support/Host.h" #include "llvm/Support/Path.h" +#include "llvm/Support/SaveAndRestore.h" #include "llvm/Support/raw_ostream.h" #include #if defined(LLVM_ON_UNIX) @@ -562,7 +563,7 @@ // Try to find an existing module with this name. if (Module *Sub = lookupModuleQualified(Name, Parent)) return std::make_pair(Sub, false); - + // Create a new module with this name. Module *Result = new Module(Name, SourceLocation(), Parent, IsFramework, IsExplicit, NumCreatedModules++); @@ -570,6 +571,8 @@ if (LangOpts.CurrentModule == Name) SourceModule = Result; Modules[Name] = Result; + if (CurrentModuleMapIsExplicitlyProvided) + ExplicitlyProvidedModules.insert(Result); } return std::make_pair(Result, true); } @@ -793,6 +796,19 @@ return Result; } +Module *ModuleMap::createShadowedModule(StringRef Name, bool IsFramework, + Module *ShadowingModule) { + + // Create a new module with this name. + Module *Result = + new Module(Name, SourceLocation(), /*Parent=*/nullptr, IsFramework, + /*IsExplicit=*/false, NumCreatedModules++); + Result->ShadowingModule = ShadowingModule; + Result->IsAvailable = false; + + return Result; +} + void ModuleMap::setUmbrellaHeader(Module *Mod, const FileEntry *UmbrellaHeader, Twine NameAsWritten) { Headers[UmbrellaHeader].push_back(KnownHeader(Mod, NormalHeader)); @@ -865,6 +881,11 @@ Mod->Headers[Module::HK_Excluded].push_back(std::move(Header)); } +void ModuleMap::setExplicitlyProvided(Module *Mod) { + assert(Modules[Mod->Name] == Mod && "explicitly provided module is shadowed"); + ExplicitlyProvidedModules.insert(Mod); +} + const FileEntry * ModuleMap::getContainingModuleMapFile(const Module *Module) const { if (Module->DefinitionLoc.isInvalid()) @@ -1462,6 +1483,7 @@ SourceLocation LBraceLoc = consumeToken(); // Determine whether this (sub)module has already been defined. + Module *ShadowingModule = nullptr; if (Module *Existing = Map.lookupModuleQualified(ModuleName, ActiveModule)) { if (Existing->DefinitionLoc.isInvalid() && !ActiveModule) { // Skip the module definition. @@ -1475,23 +1497,34 @@ } return; } - - Diags.Report(ModuleNameLoc, diag::err_mmap_module_redefinition) - << ModuleName; - Diags.Report(Existing->DefinitionLoc, diag::note_mmap_prev_definition); - - // Skip the module definition. - skipUntil(MMToken::RBrace); - if (Tok.is(MMToken::RBrace)) - consumeToken(); - - HadError = true; - return; + + if (!Existing->Parent && Map.mayShadowModuleBeingParsed(Existing)) { + ShadowingModule = Existing; + } else { + // This is not a shawdowed module decl, it is an illegal redefinition. + Diags.Report(ModuleNameLoc, diag::err_mmap_module_redefinition) + << ModuleName; + Diags.Report(Existing->DefinitionLoc, diag::note_mmap_prev_definition); + + // Skip the module definition. + skipUntil(MMToken::RBrace); + if (Tok.is(MMToken::RBrace)) + consumeToken(); + + HadError = true; + return; + } } // Start defining this module. - ActiveModule = Map.findOrCreateModule(ModuleName, ActiveModule, Framework, - Explicit).first; + if (ShadowingModule) { + ActiveModule = + Map.createShadowedModule(ModuleName, Framework, ShadowingModule); + } else { + ActiveModule = Map.findOrCreateModule(ModuleName, ActiveModule, Framework, + Explicit).first; + } + ActiveModule->DefinitionLoc = ModuleNameLoc; if (Attrs.IsSystem || IsSystem) ActiveModule->IsSystem = true; @@ -2524,7 +2557,8 @@ bool ModuleMap::parseModuleMapFile(const FileEntry *File, bool IsSystem, const DirectoryEntry *Dir, - SourceLocation ExternModuleLoc) { + SourceLocation ExternModuleLoc, + bool IsExplicitlyProvided) { llvm::DenseMap::iterator Known = ParsedModuleMap.find(File); if (Known != ParsedModuleMap.end()) @@ -2537,6 +2571,9 @@ if (!Buffer) return ParsedModuleMap[File] = true; + llvm::SaveAndRestore OldExplicit(CurrentModuleMapIsExplicitlyProvided); + CurrentModuleMapIsExplicitlyProvided |= IsExplicitlyProvided; + // Parse this module map file. Lexer L(ID, SourceMgr.getBuffer(ID), SourceMgr, MMapLangOpts); SourceLocation Start = L.getSourceLocation(); Index: lib/Lex/PPDirectives.cpp =================================================================== --- lib/Lex/PPDirectives.cpp +++ lib/Lex/PPDirectives.cpp @@ -1869,13 +1869,17 @@ if (!SuggestedModule.getModule()->isAvailable()) { Module::Requirement Requirement; Module::UnresolvedHeaderDirective MissingHeader; + Module *ShadowingModule = nullptr; Module *M = SuggestedModule.getModule(); // Identify the cause. (void)M->isAvailable(getLangOpts(), getTargetInfo(), Requirement, - MissingHeader); + MissingHeader, ShadowingModule); if (MissingHeader.FileNameLoc.isValid()) { Diag(MissingHeader.FileNameLoc, diag::err_module_header_missing) << MissingHeader.IsUmbrella << MissingHeader.FileName; + } else if (ShadowingModule) { + Diag(M->DefinitionLoc, diag::err_module_shadowed) << M->Name; + Diag(ShadowingModule->DefinitionLoc, diag::note_previous_definition); } else { Diag(M->DefinitionLoc, diag::err_module_unavailable) << M->getFullModuleName() << Requirement.second << Requirement.first; Index: test/Modules/Inputs/shadow/A1/A.h =================================================================== --- /dev/null +++ test/Modules/Inputs/shadow/A1/A.h @@ -0,0 +1 @@ +#define A1_A_h Index: test/Modules/Inputs/shadow/A1/module.modulemap =================================================================== --- /dev/null +++ test/Modules/Inputs/shadow/A1/module.modulemap @@ -0,0 +1,5 @@ +module A { + header "A.h" +} + +module A1 {} Index: test/Modules/Inputs/shadow/A2/A.h =================================================================== --- /dev/null +++ test/Modules/Inputs/shadow/A2/A.h @@ -0,0 +1 @@ +#define A2_A_h Index: test/Modules/Inputs/shadow/A2/module.modulemap =================================================================== --- /dev/null +++ test/Modules/Inputs/shadow/A2/module.modulemap @@ -0,0 +1,5 @@ +module A { + header "A.h" +} + +module A2 {} Index: test/Modules/shadow.m =================================================================== --- /dev/null +++ test/Modules/shadow.m @@ -0,0 +1,21 @@ +// RUN: rm -rf %t +// RUN: not %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/shadow/A1 -I %S/Inputs/shadow/A2 %s -fsyntax-only 2>&1 | FileCheck %s -check-prefix=REDEFINITION +// RUN: not %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -fmodule-map-file=%S/Inputs/shadow/A1/module.modulemap -fmodule-map-file=%S/Inputs/shadow/A2/module.modulemap %s -fsyntax-only 2>&1 | FileCheck %s -check-prefix=REDEFINITION +// REDEFINITION: error: redefinition of module 'A' +// REDEFINITION: note: previously defined + +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -fmodule-map-file=%S/Inputs/shadow/A1/module.modulemap -I %S/Inputs/shadow %s -verify + +@import A1; +@import A2; +@import A; + +#import "A2/A.h" // expected-note {{implicitly imported}} +// expected-error@A2/module.modulemap:1 {{import of shadowed module 'A'}} +// expected-note@A1/module.modulemap:1 {{previous definition}} + +#if defined(A2_A_h) +#error got the wrong definition of module A +#elif !defined(A1_A_h) +#error missing definition from A1 +#endif