Skip to content

Commit 23453ed

Browse files
committed
Under -enable-library-evolution, imports need Library Evolution too (#25549)
Otherwise, there's no guarantee of binary compatibility, and whoever turned on library evolution support shouldn't be lulled into a false sense of security. This is just a warning for now, but will be promoted to an error later once clients have shaken out any places where they're doing this. Note that the still-experimental '@_implementationOnly' opts out of this check, because that enforces that the import doesn't make its way into the current module's public source or binary interface. rdar://50261171 (cherry picked from commit 2397bf4)
1 parent cd63c18 commit 23453ed

File tree

7 files changed

+43
-12
lines changed

7 files changed

+43
-12
lines changed

include/swift/AST/Decl.h

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -602,7 +602,7 @@ class alignas(1 << DeclAlignInBits) Decl {
602602
HasAnyUnavailableValues : 1
603603
);
604604

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

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

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

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

626-
// Whether the module is a system module.
627-
IsSystemModule : 1
626+
/// Whether the module is a system module.
627+
IsSystemModule : 1,
628+
629+
/// Whether the module was imported from Clang (or, someday, maybe another
630+
/// language).
631+
IsNonSwiftModule : 1
628632
);
629633

630634
SWIFT_INLINE_BITFIELD(PrecedenceGroupDecl, Decl, 1+2,

include/swift/AST/DiagnosticsSema.def

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -776,6 +776,11 @@ ERROR(module_not_compiled_for_private_import,none,
776776
ERROR(import_implementation_cannot_be_exported,none,
777777
"module %0 cannot be both exported and implementation-only", (Identifier))
778778

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

780785
// Operator decls
781786
ERROR(ambiguous_operator_decls,none,

include/swift/AST/Module.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,18 @@ class ModuleDecl : public DeclContext, public TypeDecl {
334334
Bits.ModuleDecl.IsSystemModule = flag;
335335
}
336336

337+
/// Returns true if this module is a non-Swift module that was imported into
338+
/// Swift.
339+
///
340+
/// Right now that's just Clang modules.
341+
bool isNonSwiftModule() const {
342+
return Bits.ModuleDecl.IsNonSwiftModule;
343+
}
344+
/// \see #isNonSwiftModule
345+
void setIsNonSwiftModule(bool flag = true) {
346+
Bits.ModuleDecl.IsNonSwiftModule = flag;
347+
}
348+
337349
bool isResilient() const {
338350
return getResilienceStrategy() != ResilienceStrategy::Default;
339351
}

lib/ClangImporter/ClangImporter.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1673,8 +1673,7 @@ ModuleDecl *ClangImporter::Implementation::finishLoadingClangModule(
16731673
Identifier name = SwiftContext.getIdentifier((*clangModule).Name);
16741674
result = ModuleDecl::create(name, SwiftContext);
16751675
result->setIsSystemModule(clangModule->IsSystem);
1676-
// Silence error messages about testably importing a Clang module.
1677-
result->setTestingEnabled();
1676+
result->setIsNonSwiftModule();
16781677
result->setHasResolvedImports();
16791678

16801679
wrapperUnit =
@@ -1843,8 +1842,7 @@ ClangModuleUnit *ClangImporter::Implementation::getWrapperForModule(
18431842
Identifier name = SwiftContext.getIdentifier(underlying->Name);
18441843
auto wrapper = ModuleDecl::create(name, SwiftContext);
18451844
wrapper->setIsSystemModule(underlying->IsSystem);
1846-
// Silence error messages about testably importing a Clang module.
1847-
wrapper->setTestingEnabled();
1845+
wrapper->setIsNonSwiftModule();
18481846
wrapper->setHasResolvedImports();
18491847

18501848
auto file = new (SwiftContext) ClangModuleUnit(*wrapper, *this,

lib/DWARFImporter/DWARFImporter.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,8 +120,7 @@ class DWARFImporter::Implementation {
120120
return it->second->getParentModule();
121121

122122
auto *decl = ModuleDecl::create(name, SwiftContext);
123-
// Silence error messages about testably importing a Clang module.
124-
decl->setTestingEnabled();
123+
decl->setIsNonSwiftModule();
125124
decl->setHasResolvedImports();
126125
auto wrapperUnit = new (SwiftContext) DWARFModuleUnit(*decl);
127126
ModuleWrappers.insert({name, wrapperUnit});

lib/Sema/NameBinding.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,7 @@ void NameBinder::addImport(
225225

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

248+
if (SF.getParentModule()->isResilient() &&
249+
!topLevelModule->isResilient() &&
250+
!topLevelModule->isNonSwiftModule() &&
251+
!ID->getAttrs().hasAttribute<ImplementationOnlyAttr>()) {
252+
diagnose(ID->getModulePath().front().second,
253+
diag::module_not_compiled_with_library_evolution,
254+
topLevelModule->getName(), SF.getParentModule()->getName());
255+
}
256+
247257
ImportOptions options;
248258
if (ID->isExported())
249259
options |= SourceFile::ImportFlags::Exported;

test/ParseableInterface/imports.swift

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
// RUN: %empty-directory(%t)
22
// RUN: %target-swift-frontend -emit-module -o %t/empty.swiftmodule %S/../Inputs/empty.swift
3+
// RUN: %target-swift-frontend -emit-module -o %t/emptyButWithLibraryEvolution.swiftmodule %S/../Inputs/empty.swift -enable-library-evolution
34
// 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
45

56

6-
@_exported import empty
7+
@_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}}
8+
@_exported import emptyButWithLibraryEvolution
79
import B.B2
810
import func C.c // expected-warning {{scoped imports are not yet supported in module interfaces}}
911
import D
@@ -23,4 +25,5 @@ import NotSoSecret2 // expected-warning {{'NotSoSecret2' inconsistently imported
2325
// CHECK-NEXT: {{^}}import NotSoSecret2{{$}}
2426
// CHECK-NEXT: {{^}}import Swift{{$}}
2527
// CHECK-NEXT: {{^}}@_exported import empty{{$}}
28+
// CHECK-NEXT: {{^}}@_exported import emptyButWithLibraryEvolution{{$}}
2629
// CHECK-NOT: import

0 commit comments

Comments
 (0)