diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 4b3602e5cb6..225ec272736 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -585,6 +585,9 @@ ERROR(serialization_missing_single_dependency,Fatal, "missing required module '%0'", (StringRef)) ERROR(serialization_missing_dependencies,Fatal, "missing required modules: %0", (StringRef)) +ERROR(serialization_circular_dependency,Fatal, + "circular dependency between modules '%0' and %1", + (StringRef, Identifier)) ERROR(serialization_missing_shadowed_module,Fatal, "cannot load underlying module for %0", (Identifier)) ERROR(serialization_name_mismatch,Fatal, diff --git a/include/swift/AST/Module.h b/include/swift/AST/Module.h index 99386efc9bc..91f301ff047 100644 --- a/include/swift/AST/Module.h +++ b/include/swift/AST/Module.h @@ -206,6 +206,7 @@ private: unsigned TestingEnabled : 1; unsigned FailedToLoad : 1; unsigned ResilienceStrategy : 1; + unsigned HasResolvedImports : 1; } Flags; ModuleDecl(Identifier name, ASTContext &ctx); @@ -257,6 +258,13 @@ public: Flags.FailedToLoad = failed; } + bool hasResolvedImports() const { + return Flags.HasResolvedImports; + } + void setHasResolvedImports() { + Flags.HasResolvedImports = true; + } + ResilienceStrategy getResilienceStrategy() const { return ResilienceStrategy(Flags.ResilienceStrategy); } diff --git a/include/swift/Serialization/Validation.h b/include/swift/Serialization/Validation.h index dbd86e1b73c..1942d5e3e77 100644 --- a/include/swift/Serialization/Validation.h +++ b/include/swift/Serialization/Validation.h @@ -43,6 +43,10 @@ enum class Status { /// The module file is an overlay for a Clang module, which can't be found. MissingShadowedModule, + /// The module file depends on a module that is still being loaded, i.e. + /// there is a circular dependency. + CircularDependency, + /// The module file depends on a bridging header that can't be loaded. FailedToLoadBridgingHeader, diff --git a/lib/AST/ASTContext.cpp b/lib/AST/ASTContext.cpp index 0b10ea53a3e..bdbcd14ae6f 100644 --- a/lib/AST/ASTContext.cpp +++ b/lib/AST/ASTContext.cpp @@ -428,6 +428,7 @@ ConstraintCheckerArenaRAII::~ConstraintCheckerArenaRAII() { static ModuleDecl *createBuiltinModule(ASTContext &ctx) { auto M = ModuleDecl::create(ctx.getIdentifier(BUILTIN_NAME), ctx); M->addFile(*new (ctx) BuiltinUnit(*M)); + M->setHasResolvedImports(); return M; } diff --git a/lib/AST/Module.cpp b/lib/AST/Module.cpp index c5f1377b94c..c954f6fc31d 100644 --- a/lib/AST/Module.cpp +++ b/lib/AST/Module.cpp @@ -354,7 +354,7 @@ void SourceLookupCache::invalidate() { ModuleDecl::ModuleDecl(Identifier name, ASTContext &ctx) : DeclContext(DeclContextKind::Module, nullptr), TypeDecl(DeclKind::Module, &ctx, name, SourceLoc(), { }), - Flags({0, 0, 0}) { + Flags() { ctx.addDestructorCleanup(*this); setImplicit(); setInterfaceType(ModuleType::get(this)); diff --git a/lib/ClangImporter/ClangImporter.cpp b/lib/ClangImporter/ClangImporter.cpp index 6962aa3e15c..8424b7226c3 100644 --- a/lib/ClangImporter/ClangImporter.cpp +++ b/lib/ClangImporter/ClangImporter.cpp @@ -1086,6 +1086,7 @@ ClangImporter::create(ASTContext &ctx, importer->Impl.ImportedHeaderUnit = new (ctx) ClangModuleUnit(*importedHeaderModule, importer->Impl, nullptr); importedHeaderModule->addFile(*importer->Impl.ImportedHeaderUnit); + importedHeaderModule->setHasResolvedImports(); importer->Impl.IsReadingBridgingPCH = false; @@ -1591,6 +1592,7 @@ ModuleDecl *ClangImporter::Implementation::finishLoadingClangModule( result = ModuleDecl::create(name, SwiftContext); // Silence error messages about testably importing a Clang module. result->setTestingEnabled(); + result->setHasResolvedImports(); wrapperUnit = new (SwiftContext) ClangModuleUnit(*result, *this, clangModule); @@ -1735,6 +1737,7 @@ ClangModuleUnit *ClangImporter::Implementation::getWrapperForModule( auto wrapper = ModuleDecl::create(name, SwiftContext); // Silence error messages about testably importing a Clang module. wrapper->setTestingEnabled(); + wrapper->setHasResolvedImports(); auto file = new (SwiftContext) ClangModuleUnit(*wrapper, *this, underlying); diff --git a/lib/Frontend/Frontend.cpp b/lib/Frontend/Frontend.cpp index de8dd869661..dea81304d42 100644 --- a/lib/Frontend/Frontend.cpp +++ b/lib/Frontend/Frontend.cpp @@ -614,6 +614,14 @@ void CompilerInstance::parseAndCheckTypes( TypeCheckOptions); } + assert(llvm::all_of(MainModule->getFiles(), [](const FileUnit *File) -> bool { + auto *SF = dyn_cast(File); + if (!SF) + return true; + return SF->ASTStage >= SourceFile::NameBound; + }) && "some files have not yet had their imports resolved"); + MainModule->setHasResolvedImports(); + const auto &options = Invocation.getFrontendOptions(); forEachFileToTypeCheck([&](SourceFile &SF) { performTypeChecking(SF, PersistentState.getTopLevelContext(), diff --git a/lib/Sema/SourceLoader.cpp b/lib/Sema/SourceLoader.cpp index 2f69cec673b..73392811a18 100644 --- a/lib/Sema/SourceLoader.cpp +++ b/lib/Sema/SourceLoader.cpp @@ -155,6 +155,7 @@ ModuleDecl *SourceLoader::loadModule(SourceLoc importLoc, else performTypeChecking(*importFile, persistentState.getTopLevelContext(), None); + importMod->setHasResolvedImports(); return importMod; } diff --git a/lib/Serialization/ModuleFile.cpp b/lib/Serialization/ModuleFile.cpp index a1309226ca8..e889840e7b7 100644 --- a/lib/Serialization/ModuleFile.cpp +++ b/lib/Serialization/ModuleFile.cpp @@ -1439,6 +1439,13 @@ Status ModuleFile::associateWithFileContext(FileUnit *file, dependency.Import = {ctx.AllocateCopy(llvm::makeArrayRef(accessPathElem)), module}; } + + if (!module->hasResolvedImports()) { + // Notice that we check this condition /after/ recording the module that + // caused the problem. Clients need to be able to track down what the + // cycle was. + return error(Status::CircularDependency); + } } if (missingDependency) { diff --git a/lib/Serialization/SerializedModuleLoader.cpp b/lib/Serialization/SerializedModuleLoader.cpp index 5e0ed2c6934..d7f8729b454 100644 --- a/lib/Serialization/SerializedModuleLoader.cpp +++ b/lib/Serialization/SerializedModuleLoader.cpp @@ -15,6 +15,7 @@ #include "swift/Strings.h" #include "swift/AST/ASTContext.h" #include "swift/AST/DiagnosticsSema.h" +#include "swift/Basic/Defer.h" #include "swift/Basic/STLExtras.h" #include "swift/Basic/SourceManager.h" #include "swift/Basic/Version.h" @@ -316,6 +317,25 @@ FileUnit *SerializedModuleLoader::loadAST( break; } + case serialization::Status::CircularDependency: { + auto circularDependencyIter = + llvm::find_if(loadedModuleFile->getDependencies(), + [](const ModuleFile::Dependency &next) { + return !next.Import.second->hasResolvedImports(); + }); + assert(circularDependencyIter != loadedModuleFile->getDependencies().end() + && "circular dependency reported, but no module with unresolved " + "imports found"); + + // FIXME: We should include the path of the circularity as well, but that's + // hard because we're discovering this /while/ resolving imports, which + // means the problematic modules haven't been recorded yet. + Ctx.Diags.diagnose(*diagLoc, diag::serialization_circular_dependency, + circularDependencyIter->getPrettyPrintedPath(), + M.getName()); + break; + } + case serialization::Status::MissingShadowedModule: { Ctx.Diags.diagnose(*diagLoc, diag::serialization_missing_shadowed_module, M.getName()); @@ -434,6 +454,7 @@ ModuleDecl *SerializedModuleLoader::loadModule(SourceLoc importLoc, auto M = ModuleDecl::create(moduleID.first, Ctx); Ctx.LoadedModules[moduleID.first] = M; + SWIFT_DEFER { M->setHasResolvedImports(); }; if (!loadAST(*M, moduleID.second, std::move(moduleInputBuffer), std::move(moduleDocInputBuffer), isFramework)) { diff --git a/test/Serialization/circular-import.swift b/test/Serialization/circular-import.swift new file mode 100644 index 00000000000..1fbaa5ddf99 --- /dev/null +++ b/test/Serialization/circular-import.swift @@ -0,0 +1,34 @@ +// Circularities involving the module currently being type-checked. + +// RUN: %empty-directory(%t) +// RUN: %target-swift-frontend -emit-module %s -module-name A -o %t +// RUN: %target-swift-frontend -emit-module %s -module-name B -D IMPORT_A -I %t -o %t +// RUN: not %target-swift-frontend -typecheck %s -module-name A -D IMPORT_B -I %t 2>&1 | %FileCheck -check-prefix CHECK-ABA %s + +// RUN: %target-swift-frontend -emit-module %s -module-name C -D IMPORT_B -I %t -o %t +// RUN: not %target-swift-frontend -typecheck %s -module-name A -D IMPORT_C -I %t 2>&1 | %FileCheck -check-prefix CHECK-ABCA %s + +// Circularities not involving the module currently being type-checked. +// RUN: %empty-directory(%t/plain) +// RUN: %target-swift-frontend -emit-module %s -module-name A -o %t/plain +// RUN: %target-swift-frontend -emit-module %s -module-name B -o %t/plain +// RUN: %empty-directory(%t/cycle) +// RUN: %target-swift-frontend -emit-module %s -module-name A -D IMPORT_B -o %t/cycle -I %t/plain +// RUN: %target-swift-frontend -emit-module %s -module-name B -D IMPORT_A -o %t/cycle -I %t/plain +// RUN: not %target-swift-frontend -typecheck %s -module-name C -D IMPORT_A -I %t/cycle 2>&1 | %FileCheck -check-prefix CHECK-ABA-BUILT %s + + +#if IMPORT_A +import A +// CHECK-ABA-BUILT: :0: error: circular dependency between modules 'A' and 'B' +#endif + +#if IMPORT_B +import B +// CHECK-ABA: :[[@LINE-1]]:8: error: circular dependency between modules 'A' and 'B' +#endif + +#if IMPORT_C +import C +// CHECK-ABCA: :0: error: circular dependency between modules 'A' and 'B' +#endif diff --git a/test/SourceKit/Indexing/Inputs/cycle-depend/A.response b/test/SourceKit/Indexing/Inputs/cycle-depend/A.response deleted file mode 100644 index 09d54b3e2db..00000000000 --- a/test/SourceKit/Indexing/Inputs/cycle-depend/A.response +++ /dev/null @@ -1,105 +0,0 @@ -{ - key.hash: , - key.dependencies: [ - { - key.kind: source.lang.swift.import.module.swift, - key.name: "B", - key.filepath: B.swiftmodule, - key.hash: , - key.dependencies: [ - { - key.kind: source.lang.swift.import.module.swift, - key.name: "A", - key.filepath: A.swiftmodule, - key.hash: , - key.dependencies: [ - { - key.kind: source.lang.swift.import.module.swift, - key.name: "B", - key.filepath: B.swiftmodule, - key.hash: - }, - { - key.kind: source.lang.swift.import.module.swift, - key.name: "Swift", - key.filepath: Swift.swiftmodule, - key.hash: , - key.is_system: 1 - }, - { - key.kind: source.lang.swift.import.module.swift, - key.name: "SwiftOnoneSupport", - key.filepath: SwiftOnoneSupport.swiftmodule, - key.hash: , - key.dependencies: [ - { - key.kind: source.lang.swift.import.module.swift, - key.name: "Swift", - key.filepath: Swift.swiftmodule, - key.hash: , - key.is_system: 1 - } - ] - } - ] - }, - { - key.kind: source.lang.swift.import.module.swift, - key.name: "Swift", - key.filepath: Swift.swiftmodule, - key.hash: , - key.is_system: 1 - }, - { - key.kind: source.lang.swift.import.module.swift, - key.name: "SwiftOnoneSupport", - key.filepath: SwiftOnoneSupport.swiftmodule, - key.hash: - } - ] - }, - { - key.kind: source.lang.swift.import.module.swift, - key.name: "Swift", - key.filepath: Swift.swiftmodule, - key.hash: , - key.is_system: 1 - }, - { - key.kind: source.lang.swift.import.module.swift, - key.name: "SwiftOnoneSupport", - key.filepath: SwiftOnoneSupport.swiftmodule, - key.hash: - } - ], - key.entities: [ - { - key.kind: source.lang.swift.decl.class, - key.name: "A", - key.usr: "s:1AAAC", - key.entities: [ - { - key.kind: source.lang.swift.decl.var.instance, - key.name: "x", - key.usr: "s:1AAAC1x1BADCvp", - key.entities: [ - { - key.kind: source.lang.swift.decl.function.accessor.getter, - key.usr: "s:1AAAC1x1BADCvg", - key.is_dynamic: 1 - }, - { - key.kind: source.lang.swift.decl.function.accessor.setter, - key.usr: "s:1AAAC1x1BADCvs", - key.is_dynamic: 1 - } - ] - }, - { - key.kind: source.lang.swift.decl.function.constructor, - key.usr: "s:1AAACABycfc" - } - ] - } - ] -} diff --git a/test/SourceKit/Indexing/Inputs/cycle-depend/A.swift b/test/SourceKit/Indexing/Inputs/cycle-depend/A.swift deleted file mode 100644 index b15d3a3da98..00000000000 --- a/test/SourceKit/Indexing/Inputs/cycle-depend/A.swift +++ /dev/null @@ -1,5 +0,0 @@ -import B - -public class A { - var x: B = B() -} diff --git a/test/SourceKit/Indexing/Inputs/cycle-depend/B.swift b/test/SourceKit/Indexing/Inputs/cycle-depend/B.swift deleted file mode 100644 index 96ff9e05e86..00000000000 --- a/test/SourceKit/Indexing/Inputs/cycle-depend/B.swift +++ /dev/null @@ -1,6 +0,0 @@ -import A - -public class B { - var x: A? - public init() {} -} diff --git a/test/SourceKit/Indexing/index_module_cycle.swift b/test/SourceKit/Indexing/index_module_cycle.swift deleted file mode 100644 index 71f69805336..00000000000 --- a/test/SourceKit/Indexing/index_module_cycle.swift +++ /dev/null @@ -1,6 +0,0 @@ -// RUN: %empty-directory(%t) -// RUN: %swift -emit-module -o %t %S/Inputs/cycle-depend/A.swift -I %S/Inputs/cycle-depend -enable-source-import -// RUN: %swift -emit-module -o %t %S/Inputs/cycle-depend/B.swift -I %S/Inputs/cycle-depend -enable-source-import - -// RUN: %sourcekitd-test -req=index %t/A.swiftmodule -- %t/A.swiftmodule -I %t | %sed_clean > %t.response -// RUN: diff -u %S/Inputs/cycle-depend/A.response %t.response diff --git a/test/SourceKit/Indexing/index_module_missing_depend.swift b/test/SourceKit/Indexing/index_module_missing_depend.swift index 4848e586103..3ff0101abf8 100644 --- a/test/SourceKit/Indexing/index_module_missing_depend.swift +++ b/test/SourceKit/Indexing/index_module_missing_depend.swift @@ -1,8 +1,12 @@ // RUN: %empty-directory(%t) -// RUN: %swift -emit-module -o %t %S/Inputs/cycle-depend/A.swift -I %S/Inputs/cycle-depend -enable-source-import +// RUN: %target-swift-frontend -emit-module -o %t %S/../../Inputs/empty.swift +// RUN: %target-swift-frontend -emit-module -o %t %s -I %t -module-name A +// RUN: rm %t/empty.swiftmodule // RUN: not %sourcekitd-test -req=index %t/A.swiftmodule -- %t/A.swiftmodule 2>&1 | %FileCheck %s +import empty + // FIXME: Report the reason we couldn't load a module. // CHECK-DISABLED: error response (Request Failed): missing module dependency // CHECK: error response (Request Failed): failed to load module diff --git a/tools/SourceKit/lib/SwiftLang/SwiftIndexing.cpp b/tools/SourceKit/lib/SwiftLang/SwiftIndexing.cpp index 0240b10d7c1..c8407eef55b 100644 --- a/tools/SourceKit/lib/SwiftLang/SwiftIndexing.cpp +++ b/tools/SourceKit/lib/SwiftLang/SwiftIndexing.cpp @@ -184,6 +184,8 @@ static void indexModule(llvm::MemoryBuffer *Input, IdxConsumer.failed("failed to load module"); return; } + + Mod->setHasResolvedImports(); } // Setup a typechecker for protocol conformance resolving.