Skip to content

Under -enable-library-evolution, imports need Library Evolution too #25549

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 18, 2019
Merged

Under -enable-library-evolution, imports need Library Evolution too #25549

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions include/swift/AST/Decl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,7 @@ class alignas(1 << DeclAlignInBits) Decl {
HasAnyUnavailableValues : 1
);

SWIFT_INLINE_BITFIELD(ModuleDecl, TypeDecl, 1+1+1+1+1+1+1,
SWIFT_INLINE_BITFIELD(ModuleDecl, TypeDecl, 1+1+1+1+1+1+1+1,
/// If the module was or is being compiled with `-enable-testing`.
TestingEnabled : 1,

Expand All @@ -620,14 +620,18 @@ class alignas(1 << DeclAlignInBits) Decl {
/// Whether all imports have been resolved. Used to detect circular imports.
HasResolvedImports : 1,

// If the module was or is being compiled with `-enable-private-imports`.
/// If the module was or is being compiled with `-enable-private-imports`.
PrivateImportsEnabled : 1,

// If the module is compiled with `-enable-implicit-dynamic`.
/// If the module is compiled with `-enable-implicit-dynamic`.
ImplicitDynamicEnabled : 1,

// Whether the module is a system module.
IsSystemModule : 1
/// Whether the module is a system module.
IsSystemModule : 1,

/// Whether the module was imported from Clang (or, someday, maybe another
/// language).
IsNonSwiftModule : 1
);

SWIFT_INLINE_BITFIELD(PrecedenceGroupDecl, Decl, 1+2,
Expand Down
5 changes: 5 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -792,6 +792,11 @@ ERROR(module_not_compiled_for_private_import,none,
ERROR(import_implementation_cannot_be_exported,none,
"module %0 cannot be both exported and implementation-only", (Identifier))

WARNING(module_not_compiled_with_library_evolution,none,
"module %0 was not compiled with library evolution support; "
"using it means binary compatibility for %1 can't be guaranteed",
(Identifier, Identifier))


// Operator decls
ERROR(ambiguous_operator_decls,none,
Expand Down
12 changes: 12 additions & 0 deletions include/swift/AST/Module.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,18 @@ class ModuleDecl : public DeclContext, public TypeDecl {
Bits.ModuleDecl.IsSystemModule = flag;
}

/// Returns true if this module is a non-Swift module that was imported into
/// Swift.
///
/// Right now that's just Clang modules.
bool isNonSwiftModule() const {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm planning follow-up work where I check on all uses of findUnderlyingClangModule() that don't actually need to check more than what this predicate is doing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oof, ModuleDecl::isClangModule already exists and I just didn't spot it. This is still a better implementation, but maybe I'll give up on my too-forward-looking name.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

…and the one use of isClangModule is incorrect, because it looks through overlays.

return Bits.ModuleDecl.IsNonSwiftModule;
}
/// \see #isNonSwiftModule
void setIsNonSwiftModule(bool flag = true) {
Bits.ModuleDecl.IsNonSwiftModule = flag;
}

bool isResilient() const {
return getResilienceStrategy() != ResilienceStrategy::Default;
}
Expand Down
6 changes: 2 additions & 4 deletions lib/ClangImporter/ClangImporter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1672,8 +1672,7 @@ ModuleDecl *ClangImporter::Implementation::finishLoadingClangModule(
Identifier name = SwiftContext.getIdentifier((*clangModule).Name);
result = ModuleDecl::create(name, SwiftContext);
result->setIsSystemModule(clangModule->IsSystem);
// Silence error messages about testably importing a Clang module.
result->setTestingEnabled();
result->setIsNonSwiftModule();
result->setHasResolvedImports();

wrapperUnit =
Expand Down Expand Up @@ -1842,8 +1841,7 @@ ClangModuleUnit *ClangImporter::Implementation::getWrapperForModule(
Identifier name = SwiftContext.getIdentifier(underlying->Name);
auto wrapper = ModuleDecl::create(name, SwiftContext);
wrapper->setIsSystemModule(underlying->IsSystem);
// Silence error messages about testably importing a Clang module.
wrapper->setTestingEnabled();
wrapper->setIsNonSwiftModule();
wrapper->setHasResolvedImports();

auto file = new (SwiftContext) ClangModuleUnit(*wrapper, *this,
Expand Down
3 changes: 1 addition & 2 deletions lib/DWARFImporter/DWARFImporter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,7 @@ class DWARFImporter::Implementation {
return it->second->getParentModule();

auto *decl = ModuleDecl::create(name, SwiftContext);
// Silence error messages about testably importing a Clang module.
decl->setTestingEnabled();
decl->setIsNonSwiftModule();
decl->setHasResolvedImports();
auto wrapperUnit = new (SwiftContext) DWARFModuleUnit(*decl);
ModuleWrappers.insert({name, wrapperUnit});
Expand Down
10 changes: 10 additions & 0 deletions lib/Sema/NameBinding.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@ void NameBinder::addImport(

auto *testableAttr = ID->getAttrs().getAttribute<TestableAttr>();
if (testableAttr && !topLevelModule->isTestingEnabled() &&
!topLevelModule->isNonSwiftModule() &&
Context.LangOpts.EnableTestableAttrRequiresTestableModule) {
diagnose(ID->getModulePath().front().second, diag::module_not_testable,
topLevelModule->getName());
Expand All @@ -244,6 +245,15 @@ void NameBinder::addImport(
}
}

if (SF.getParentModule()->isResilient() &&
!topLevelModule->isResilient() &&
!topLevelModule->isNonSwiftModule() &&
!ID->getAttrs().hasAttribute<ImplementationOnlyAttr>()) {
diagnose(ID->getModulePath().front().second,
diag::module_not_compiled_with_library_evolution,
topLevelModule->getName(), SF.getParentModule()->getName());
}

ImportOptions options;
if (ID->isExported())
options |= SourceFile::ImportFlags::Exported;
Expand Down
5 changes: 4 additions & 1 deletion test/ParseableInterface/imports.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
// RUN: %empty-directory(%t)
// RUN: %target-swift-frontend -emit-module -o %t/empty.swiftmodule %S/../Inputs/empty.swift
// RUN: %target-swift-frontend -emit-module -o %t/emptyButWithLibraryEvolution.swiftmodule %S/../Inputs/empty.swift -enable-library-evolution
// RUN: %target-swift-frontend -typecheck -emit-parseable-module-interface-path - %s %S/Inputs/imports-other.swift -I %S/Inputs/imports-clang-modules/ -I %t -verify -swift-version 5 -enable-library-evolution | %FileCheck -implicit-check-not BAD %s


@_exported import empty
@_exported import empty // expected-warning {{module 'empty' was not compiled with library evolution support; using it means binary compatibility for 'imports' can't be guaranteed}}
@_exported import emptyButWithLibraryEvolution
import B.B2
import func C.c // expected-warning {{scoped imports are not yet supported in module interfaces}}
import D
Expand All @@ -23,4 +25,5 @@ import NotSoSecret2 // expected-warning {{'NotSoSecret2' inconsistently imported
// CHECK-NEXT: {{^}}import NotSoSecret2{{$}}
// CHECK-NEXT: {{^}}import Swift{{$}}
// CHECK-NEXT: {{^}}@_exported import empty{{$}}
// CHECK-NEXT: {{^}}@_exported import emptyButWithLibraryEvolution{{$}}
// CHECK-NOT: import