Skip to content

Commit

Permalink
Revert "[Modules] Allow modules specified by -fmodule-map-file to sha…
Browse files Browse the repository at this point in the history 
…dow implicitly found ones"

This reverts r321781 until I fix the leaks pointed out by bots:

http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/12146
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap/builds/3741

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@321786 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
@bcardosolopes
bcardosolopes committed Jan 4, 2018
1 parent 21f849e commit de6dd39
Show file tree
Hide file tree
Showing 23 changed files with 52 additions and 222 deletions.
3 changes: 0 additions & 3 deletions include/clang/Basic/DiagnosticCommonKinds.td
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,6 @@ def remark_module_lock_failure : Remark<
"could not acquire lock file for module '%0': %1">, InGroup<ModuleBuild>;
def remark_module_lock_timeout : Remark<
"timed out waiting to acquire lock file for module '%0'">, InGroup<ModuleBuild>;
def err_module_shadowed : Error<"import of shadowed module '%0'">, DefaultFatal;
def err_module_build_shadowed_submodule : Error<
"build a shadowed submodule '%0'">, DefaultFatal;
def err_module_cycle : Error<"cyclic dependency in module '%0': %1">,
DefaultFatal;
def err_module_prebuilt : Error<
Expand Down
18 changes: 4 additions & 14 deletions include/clang/Basic/Module.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,9 +197,6 @@ class Module {
/// will be false to indicate that this (sub)module is not available.
SmallVector<Requirement, 2> 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;

Expand Down Expand Up @@ -378,20 +375,13 @@ class Module {
///
/// \param Target The target options used for the current translation unit.
///
/// \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.
/// \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.
bool isAvailable(const LangOptions &LangOpts,
const TargetInfo &Target,
Requirement &Req,
UnresolvedHeaderDirective &MissingHeader,
Module *&ShadowingModule) const;
UnresolvedHeaderDirective &MissingHeader) const;

/// \brief Determine whether this module is a submodule.
bool isSubModule() const { return Parent != nullptr; }
Expand Down
1 change: 0 additions & 1 deletion include/clang/Lex/HeaderSearch.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -726,7 +726,6 @@ class HeaderSearch {
LoadModuleMapResult loadModuleMapFileImpl(const FileEntry *File,
bool IsSystem,
const DirectoryEntry *Dir,
bool IsExplicitlyProvided,
FileID ID = FileID(),
unsigned *Offset = nullptr);

Expand Down
33 changes: 5 additions & 28 deletions include/clang/Lex/ModuleMap.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,17 +195,6 @@ class ModuleMap {
/// header.
llvm::DenseMap<const DirectoryEntry *, Module *> 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<const Module *> ExplicitlyProvidedModules;

bool mayShadowModuleBeingParsed(Module *ExistingModule,
bool IsExplicitlyProvided) {
assert(!ExistingModule->Parent && "expected top-level module");
return !IsExplicitlyProvided &&
ExplicitlyProvidedModules.count(ExistingModule);
}

/// \brief The set of attributes that can be attached to a module.
struct Attributes {
/// \brief Whether this is a system module.
Expand Down Expand Up @@ -486,9 +475,9 @@ class ModuleMap {
///
/// \returns The found or newly-created module, along with a boolean value
/// that will be true if the module is newly-created.
std::pair<Module *, bool>
findOrCreateModule(StringRef Name, Module *Parent, bool IsFramework,
bool IsExplicit, bool UsesExplicitModuleMapFile = false);
std::pair<Module *, bool> findOrCreateModule(StringRef Name, Module *Parent,
bool IsFramework,
bool IsExplicit);

/// \brief Create a 'global module' for a C++ Modules TS module interface
/// unit.
Expand All @@ -513,11 +502,6 @@ class ModuleMap {
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.
///
Expand Down Expand Up @@ -603,8 +587,6 @@ class ModuleMap {
/// \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.
///
Expand All @@ -624,15 +606,10 @@ class ModuleMap {
/// \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,
bool IsExplicitlyProvided = false,
FileID ID = FileID(), unsigned *Offset = nullptr,
const DirectoryEntry *HomeDir, FileID ID = FileID(),
unsigned *Offset = nullptr,
SourceLocation ExternModuleLoc = SourceLocation());

/// \brief Dump the contents of the module map, for debugging purposes.
Expand Down
7 changes: 1 addition & 6 deletions lib/Basic/Module.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,16 +95,11 @@ static bool hasFeature(StringRef Feature, const LangOptions &LangOpts,

bool Module::isAvailable(const LangOptions &LangOpts, const TargetInfo &Target,
Requirement &Req,
UnresolvedHeaderDirective &MissingHeader,
Module *&ShadowingModule) const {
UnresolvedHeaderDirective &MissingHeader) 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) {
Expand Down
20 changes: 9 additions & 11 deletions lib/Lex/HeaderSearch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1367,8 +1367,7 @@ bool HeaderSearch::loadModuleMapFile(const FileEntry *File, bool IsSystem,
}
}

switch (loadModuleMapFileImpl(File, IsSystem, Dir,
/*IsExplictlyProvided=*/true, ID, Offset)) {
switch (loadModuleMapFileImpl(File, IsSystem, Dir, ID, Offset)) {
case LMM_AlreadyLoaded:
case LMM_NewlyLoaded:
return false;
Expand All @@ -1379,9 +1378,10 @@ bool HeaderSearch::loadModuleMapFile(const FileEntry *File, bool IsSystem,
llvm_unreachable("Unknown load module map result");
}

HeaderSearch::LoadModuleMapResult HeaderSearch::loadModuleMapFileImpl(
const FileEntry *File, bool IsSystem, const DirectoryEntry *Dir,
bool IsExplicitlyProvided, FileID ID, unsigned *Offset) {
HeaderSearch::LoadModuleMapResult
HeaderSearch::loadModuleMapFileImpl(const FileEntry *File, bool IsSystem,
const DirectoryEntry *Dir, FileID ID,
unsigned *Offset) {
assert(File && "expected FileEntry");

// Check whether we've already loaded this module map, and mark it as being
Expand All @@ -1390,16 +1390,14 @@ HeaderSearch::LoadModuleMapResult HeaderSearch::loadModuleMapFileImpl(
if (!AddResult.second)
return AddResult.first->second ? LMM_AlreadyLoaded : LMM_InvalidModuleMap;

if (ModMap.parseModuleMapFile(File, IsSystem, Dir, IsExplicitlyProvided, ID,
Offset)) {
if (ModMap.parseModuleMapFile(File, IsSystem, Dir, ID, Offset)) {
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,
IsExplicitlyProvided)) {
if (ModMap.parseModuleMapFile(PMMFile, IsSystem, Dir)) {
LoadedModuleMaps[File] = false;
return LMM_InvalidModuleMap;
}
Expand Down Expand Up @@ -1470,8 +1468,8 @@ HeaderSearch::loadModuleMapFile(const DirectoryEntry *Dir, bool IsSystem,
return KnownDir->second ? LMM_AlreadyLoaded : LMM_InvalidModuleMap;

if (const FileEntry *ModuleMapFile = lookupModuleMapFile(Dir, IsFramework)) {
LoadModuleMapResult Result = loadModuleMapFileImpl(
ModuleMapFile, IsSystem, Dir, /*IsExplicitlyProvided=*/false);
LoadModuleMapResult Result =
loadModuleMapFileImpl(ModuleMapFile, IsSystem, Dir);
// Add Dir explicitly in case ModuleMapFile is in a subdirectory.
// E.g. Foo.framework/Modules/module.modulemap
// ^Dir ^ModuleMapFile
Expand Down
100 changes: 32 additions & 68 deletions lib/Lex/ModuleMap.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -744,22 +744,21 @@ Module *ModuleMap::lookupModuleQualified(StringRef Name, Module *Context) const{
return Context->findSubmodule(Name);
}

std::pair<Module *, bool>
ModuleMap::findOrCreateModule(StringRef Name, Module *Parent, bool IsFramework,
bool IsExplicit, bool UsesExplicitModuleMapFile) {
std::pair<Module *, bool> ModuleMap::findOrCreateModule(StringRef Name,
Module *Parent,
bool IsFramework,
bool IsExplicit) {
// 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++);
if (!Parent) {
if (LangOpts.CurrentModule == Name)
SourceModule = Result;
Modules[Name] = Result;
if (UsesExplicitModuleMapFile)
ExplicitlyProvidedModules.insert(Result);
}
return std::make_pair(Result, true);
}
Expand Down Expand Up @@ -1000,19 +999,6 @@ Module *ModuleMap::inferFrameworkModule(const DirectoryEntry *FrameworkDir,
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));
Expand Down Expand Up @@ -1130,11 +1116,6 @@ void ModuleMap::excludeHeader(Module *Mod, Module::Header Header) {
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())
Expand Down Expand Up @@ -1338,9 +1319,7 @@ namespace clang {

/// \brief Consume the current token and return its location.
SourceLocation consumeToken();

bool UsesExplicitModuleMapFile = false;


/// \brief Skip tokens until we reach the a token with the given kind
/// (or the end of the file).
void skipUntil(MMToken::TokenKind K);
Expand All @@ -1366,19 +1345,20 @@ namespace clang {
bool parseOptionalAttributes(Attributes &Attrs);

public:
explicit ModuleMapParser(Lexer &L, SourceManager &SourceMgr,
const TargetInfo *Target, DiagnosticsEngine &Diags,
ModuleMap &Map, const FileEntry *ModuleMapFile,
const DirectoryEntry *Directory, bool IsSystem,
bool UsesExplicitModuleMapFile)
explicit ModuleMapParser(Lexer &L, SourceManager &SourceMgr,
const TargetInfo *Target,
DiagnosticsEngine &Diags,
ModuleMap &Map,
const FileEntry *ModuleMapFile,
const DirectoryEntry *Directory,
bool IsSystem)
: L(L), SourceMgr(SourceMgr), Target(Target), Diags(Diags), Map(Map),
ModuleMapFile(ModuleMapFile), Directory(Directory),
IsSystem(IsSystem),
UsesExplicitModuleMapFile(UsesExplicitModuleMapFile) {
IsSystem(IsSystem) {
Tok.clear();
consumeToken();
}

bool parseModuleMapFile();

bool terminatedByDirective() { return false; }
Expand Down Expand Up @@ -1807,7 +1787,6 @@ void ModuleMapParser::parseModuleDecl() {
SourceLocation LBraceLoc = consumeToken();

// Determine whether this (sub)module has already been defined.
Module *ShadowingModule = nullptr;
if (Module *Existing = Map.lookupModuleQualified(ModuleName, ActiveModule)) {
// We might see a (re)definition of a module that we already have a
// definition for in two cases:
Expand All @@ -1833,36 +1812,23 @@ void ModuleMapParser::parseModuleDecl() {
}
return;
}

if (!Existing->Parent &&
Map.mayShadowModuleBeingParsed(Existing, UsesExplicitModuleMapFile)) {
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;
}

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.
if (ShadowingModule) {
ActiveModule =
Map.createShadowedModule(ModuleName, Framework, ShadowingModule);
} else {
ActiveModule = Map.findOrCreateModule(ModuleName, ActiveModule, Framework,
Explicit, UsesExplicitModuleMapFile)
.first;
}

ActiveModule = Map.findOrCreateModule(ModuleName, ActiveModule, Framework,
Explicit).first;
ActiveModule->DefinitionLoc = ModuleNameLoc;
if (Attrs.IsSystem || IsSystem)
ActiveModule->IsSystem = true;
Expand Down Expand Up @@ -2038,7 +2004,7 @@ void ModuleMapParser::parseExternModuleDecl() {
Map.HeaderInfo.getHeaderSearchOpts().ModuleMapFileHomeIsCwd
? Directory
: File->getDir(),
false /*IsExplicitlyProvided*/, FileID(), nullptr, ExternLoc);
FileID(), nullptr, ExternLoc);
}

/// Whether to add the requirement \p Feature to the module \p M.
Expand Down Expand Up @@ -2845,8 +2811,7 @@ bool ModuleMapParser::parseModuleMapFile() {
}

bool ModuleMap::parseModuleMapFile(const FileEntry *File, bool IsSystem,
const DirectoryEntry *Dir,
bool IsExplicitlyProvided, FileID ID,
const DirectoryEntry *Dir, FileID ID,
unsigned *Offset,
SourceLocation ExternModuleLoc) {
assert(Target && "Missing target information");
Expand Down Expand Up @@ -2876,7 +2841,7 @@ bool ModuleMap::parseModuleMapFile(const FileEntry *File, bool IsSystem,
Buffer->getBufferEnd());
SourceLocation Start = L.getSourceLocation();
ModuleMapParser Parser(L, SourceMgr, Target, Diags, *this, File, Dir,
IsSystem, IsExplicitlyProvided);
IsSystem);
bool Result = Parser.parseModuleMapFile();
ParsedModuleMap[File] = Result;

Expand All @@ -2889,6 +2854,5 @@ bool ModuleMap::parseModuleMapFile(const FileEntry *File, bool IsSystem,
// Notify callbacks that we parsed it.
for (const auto &Cb : Callbacks)
Cb->moduleMapFileRead(Start, *File, IsSystem);

return Result;
}
Loading

0 comments on commit de6dd39

Please sign in to comment.