Keep track of modules that have failed to build. If we encounter an

import of that module elsewhere, don't try to build the module again:
it won't work, and the experience is quite dreadful. We track this
information somewhat globally, shared among all of the related
CompilerInvocations used to build modules on-the-fly, so that a
particular Clang instance will only try to build a given module once.

Fixes <rdar://problem/12552849>.

llvm-svn: 168961
This commit is contained in:
Douglas Gregor 2012-11-29 23:55:25 +00:00
parent 136d6746c5
commit 7a626570ef
7 changed files with 135 additions and 33 deletions

View File

@ -830,11 +830,12 @@ public:
/// \returns True if an error occurred, false otherwise.
bool serialize(raw_ostream &OS);
virtual Module *loadModule(SourceLocation ImportLoc, ModuleIdPath Path,
Module::NameVisibilityKind Visibility,
bool IsInclusionDirective) {
virtual ModuleLoadResult loadModule(SourceLocation ImportLoc,
ModuleIdPath Path,
Module::NameVisibilityKind Visibility,
bool IsInclusionDirective) {
// ASTUnit doesn't know how to load modules (not that this matters).
return 0;
return ModuleLoadResult();
}
};

View File

@ -94,7 +94,7 @@ class CompilerInstance : public ModuleLoader {
/// \brief The semantic analysis object.
OwningPtr<Sema> TheSema;
/// \brief The frontend timer
OwningPtr<llvm::Timer> FrontendTimer;
@ -111,7 +111,7 @@ class CompilerInstance : public ModuleLoader {
/// \brief The result of the last module import.
///
Module *LastModuleImportResult;
ModuleLoadResult LastModuleImportResult;
/// \brief Holds information about the output file.
///
@ -645,9 +645,10 @@ public:
/// }
virtual Module *loadModule(SourceLocation ImportLoc, ModuleIdPath Path,
Module::NameVisibilityKind Visibility,
bool IsInclusionDirective);
virtual ModuleLoadResult loadModule(SourceLocation ImportLoc,
ModuleIdPath Path,
Module::NameVisibilityKind Visibility,
bool IsInclusionDirective);
};
} // end namespace clang

View File

@ -17,16 +17,37 @@
#include "clang/Basic/Module.h"
#include "clang/Basic/SourceLocation.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/PointerIntPair.h"
namespace clang {
class IdentifierInfo;
class Module;
/// \brief A sequence of identifier/location pairs used to describe a particular
/// module or submodule, e.g., std.vector.
typedef llvm::ArrayRef<std::pair<IdentifierInfo*, SourceLocation> >
ModuleIdPath;
/// \brief Describes the result of attempting to load a module.
class ModuleLoadResult {
llvm::PointerIntPair<Module *, 1, bool> Storage;
public:
ModuleLoadResult() : Storage() { }
ModuleLoadResult(Module *module, bool missingExpected)
: Storage(module, missingExpected) { }
operator Module *() const { return Storage.getPointer(); }
/// \brief Determines whether the module, which failed to load, was
/// actually a submodule that we expected to see (based on implying the
/// submodule from header structure), but didn't materialize in the actual
/// module.
bool isMissingExpected() const { return Storage.getInt(); }
};
/// \brief Abstract interface for a module loader.
///
/// This abstract interface describes a module loader, which is responsible
@ -55,9 +76,10 @@ public:
///
/// \returns If successful, returns the loaded module. Otherwise, returns
/// NULL to indicate that the module could not be loaded.
virtual Module *loadModule(SourceLocation ImportLoc, ModuleIdPath Path,
Module::NameVisibilityKind Visibility,
bool IsInclusionDirective) = 0;
virtual ModuleLoadResult loadModule(SourceLocation ImportLoc,
ModuleIdPath Path,
Module::NameVisibilityKind Visibility,
bool IsInclusionDirective) = 0;
};
}

View File

@ -13,6 +13,7 @@
#include "llvm/ADT/IntrusiveRefCntPtr.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/StringSet.h"
#include <cassert>
#include <string>
#include <utility>
@ -126,7 +127,29 @@ public:
/// to do so (e.g., if on-demand module construction moves out-of-process),
/// we can add a cc1-level option to do so.
SmallVector<std::string, 2> ModuleBuildPath;
/// \brief Records the set of modules
class FailedModulesSet : public llvm::RefCountedBase<FailedModulesSet> {
llvm::StringSet<> Failed;
public:
bool hasAlreadyFailed(StringRef module) {
return Failed.count(module) > 0;
}
void addFailed(StringRef module) {
Failed.insert(module);
}
};
/// \brief The set of modules that failed to build.
///
/// This pointer will be shared among all of the compiler instances created
/// to (re)build modules, so that once a module fails to build anywhere,
/// other instances will see that the module has failed and won't try to
/// build it again.
llvm::IntrusiveRefCntPtr<FailedModulesSet> FailedModules;
typedef std::vector<std::pair<std::string, std::string> >::iterator
remapped_file_iterator;
typedef std::vector<std::pair<std::string, std::string> >::const_iterator

View File

@ -801,6 +801,15 @@ static void compileModule(CompilerInstance &ImportingInstance,
// can detect cycles in the module graph.
PPOpts.ModuleBuildPath.push_back(Module->getTopLevelModuleName());
// Make sure that the failed-module structure has been allocated in
// the importing instance, and propagate the pointer to the newly-created
// instance.
PreprocessorOptions &ImportingPPOpts
= ImportingInstance.getInvocation().getPreprocessorOpts();
if (!ImportingPPOpts.FailedModules)
ImportingPPOpts.FailedModules = new PreprocessorOptions::FailedModulesSet;
PPOpts.FailedModules = ImportingPPOpts.FailedModules;
// If there is a module map file, build the module using the module map.
// Set up the inputs/outputs so that we build the module from its umbrella
// header.
@ -872,10 +881,11 @@ static void compileModule(CompilerInstance &ImportingInstance,
llvm::sys::Path(TempModuleMapFileName).eraseFromDisk();
}
Module *CompilerInstance::loadModule(SourceLocation ImportLoc,
ModuleIdPath Path,
Module::NameVisibilityKind Visibility,
bool IsInclusionDirective) {
ModuleLoadResult
CompilerInstance::loadModule(SourceLocation ImportLoc,
ModuleIdPath Path,
Module::NameVisibilityKind Visibility,
bool IsInclusionDirective) {
// If we've already handled this import, just return the cached result.
// This one-element cache is important to eliminate redundant diagnostics
// when both the preprocessor and parser see the same import declaration.
@ -910,14 +920,14 @@ Module *CompilerInstance::loadModule(SourceLocation ImportLoc,
ModuleFileName = PP->getHeaderSearchInfo().getModuleFileName(Module);
else
ModuleFileName = PP->getHeaderSearchInfo().getModuleFileName(ModuleName);
if (ModuleFileName.empty()) {
getDiagnostics().Report(ModuleNameLoc, diag::err_module_not_found)
<< ModuleName
<< SourceRange(ImportLoc, ModuleNameLoc);
LastModuleImportLoc = ImportLoc;
LastModuleImportResult = 0;
return 0;
LastModuleImportResult = ModuleLoadResult();
return LastModuleImportResult;
}
const FileEntry *ModuleFile
@ -943,7 +953,18 @@ Module *CompilerInstance::loadModule(SourceLocation ImportLoc,
getDiagnostics().Report(ModuleNameLoc, diag::err_module_cycle)
<< ModuleName << CyclePath;
return 0;
return ModuleLoadResult();
}
// Check whether we have already attempted to build this module (but
// failed).
if (getPreprocessorOpts().FailedModules &&
getPreprocessorOpts().FailedModules->hasAlreadyFailed(ModuleName)) {
getDiagnostics().Report(ModuleNameLoc, diag::err_module_not_built)
<< ModuleName
<< SourceRange(ImportLoc, ModuleNameLoc);
return ModuleLoadResult();
}
getDiagnostics().Report(ModuleNameLoc, diag::warn_module_build)
@ -951,6 +972,9 @@ Module *CompilerInstance::loadModule(SourceLocation ImportLoc,
BuildingModule = true;
compileModule(*this, Module, ModuleFileName);
ModuleFile = FileMgr->getFile(ModuleFileName);
if (!ModuleFile)
getPreprocessorOpts().FailedModules->addFailed(ModuleName);
}
if (!ModuleFile) {
@ -959,7 +983,7 @@ Module *CompilerInstance::loadModule(SourceLocation ImportLoc,
: diag::err_module_not_found)
<< ModuleName
<< SourceRange(ImportLoc, ModuleNameLoc);
return 0;
return ModuleLoadResult();
}
// If we don't already have an ASTReader, create one now.
@ -1004,6 +1028,18 @@ Module *CompilerInstance::loadModule(SourceLocation ImportLoc,
getFileManager().invalidateCache(ModuleFile);
bool Existed;
llvm::sys::fs::remove(ModuleFileName, Existed);
// Check whether we have already attempted to build this module (but
// failed).
if (getPreprocessorOpts().FailedModules &&
getPreprocessorOpts().FailedModules->hasAlreadyFailed(ModuleName)) {
getDiagnostics().Report(ModuleNameLoc, diag::err_module_not_built)
<< ModuleName
<< SourceRange(ImportLoc, ModuleNameLoc);
return ModuleLoadResult();
}
compileModule(*this, Module, ModuleFileName);
// Try loading the module again.
@ -1012,8 +1048,9 @@ Module *CompilerInstance::loadModule(SourceLocation ImportLoc,
ModuleManager->ReadAST(ModuleFileName,
serialization::MK_Module, ImportLoc,
ASTReader::ARR_None) != ASTReader::Success) {
getPreprocessorOpts().FailedModules->addFailed(ModuleName);
KnownModules[Path[0].first] = 0;
return 0;
return ModuleLoadResult();
}
// Okay, we've rebuilt and now loaded the module.
@ -1026,12 +1063,12 @@ Module *CompilerInstance::loadModule(SourceLocation ImportLoc,
// FIXME: The ASTReader will already have complained, but can we showhorn
// that diagnostic information into a more useful form?
KnownModules[Path[0].first] = 0;
return 0;
return ModuleLoadResult();
case ASTReader::Failure:
// Already complained, but note now that we failed.
KnownModules[Path[0].first] = 0;
return 0;
return ModuleLoadResult();
}
if (!Module) {
@ -1050,7 +1087,7 @@ Module *CompilerInstance::loadModule(SourceLocation ImportLoc,
// If we never found the module, fail.
if (!Module)
return 0;
return ModuleLoadResult();
// Verify that the rest of the module path actually corresponds to
// a submodule.
@ -1120,7 +1157,7 @@ Module *CompilerInstance::loadModule(SourceLocation ImportLoc,
<< Module->getFullModuleName()
<< SourceRange(Path.front().second, Path.back().second);
return 0;
return ModuleLoadResult(0, true);
}
// Check whether this module is available.
@ -1131,8 +1168,8 @@ Module *CompilerInstance::loadModule(SourceLocation ImportLoc,
<< Feature
<< SourceRange(Path.front().second, Path.back().second);
LastModuleImportLoc = ImportLoc;
LastModuleImportResult = 0;
return 0;
LastModuleImportResult = ModuleLoadResult();
return ModuleLoadResult();
}
ModuleManager->makeModuleVisible(Module, Visibility);
@ -1151,6 +1188,6 @@ Module *CompilerInstance::loadModule(SourceLocation ImportLoc,
}
LastModuleImportLoc = ImportLoc;
LastModuleImportResult = Module;
return Module;
LastModuleImportResult = ModuleLoadResult(Module, false);
return LastModuleImportResult;
}

View File

@ -1483,7 +1483,7 @@ void Preprocessor::HandleIncludeDirective(SourceLocation HashLoc,
// If this was an #__include_macros directive, only make macros visible.
Module::NameVisibilityKind Visibility
= (IncludeKind == 3)? Module::MacrosVisible : Module::AllVisible;
Module *Imported
ModuleLoadResult Imported
= TheModuleLoader.loadModule(IncludeTok.getLocation(), Path, Visibility,
/*IsIncludeDirective=*/true);
assert((Imported == 0 || Imported == SuggestedModule) &&
@ -1498,6 +1498,13 @@ void Preprocessor::HandleIncludeDirective(SourceLocation HashLoc,
}
return;
}
// If we failed to find a submodule that we expected to find, we can
// continue. Otherwise, there's an error in the included file, so we
// don't want to include it.
if (!BuildingImportedModule && !Imported.isMissingExpected()) {
return;
}
}
if (Callbacks && SuggestedModule) {

View File

@ -0,0 +1,11 @@
// RUN: rm -rf %t
// RUN: %clang_cc1 -fmodule-cache-path %t -fmodules -F %S/Inputs -DgetModuleVersion="epic fail" %s 2>&1 | FileCheck %s
@__experimental_modules_import Module;
@__experimental_modules_import DependsOnModule;
// CHECK: error: expected ';' after top level declarator
// CHECK: note: expanded from macro 'getModuleVersion'
// CHECK: fatal error: could not build module 'Module'
// CHECK: fatal error: could not build module 'Module'
// CHECK-NOT: error: