Skip to content

Commit 2397bf4

Browse files
authored
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
1 parent 85bfbe2 commit 2397bf4

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
@@ -605,7 +605,7 @@ class alignas(1 << DeclAlignInBits) Decl {
605605
HasAnyUnavailableValues : 1
606606
);
607607

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

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

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

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

629-
// Whether the module is a system module.
630-
IsSystemModule : 1
629+
/// Whether the module is a system module.
630+
IsSystemModule : 1,
631+
632+
/// Whether the module was imported from Clang (or, someday, maybe another
633+
/// language).
634+
IsNonSwiftModule : 1
631635
);
632636

633637
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
@@ -792,6 +792,11 @@ ERROR(module_not_compiled_for_private_import,none,
792792
ERROR(import_implementation_cannot_be_exported,none,
793793
"module %0 cannot be both exported and implementation-only", (Identifier))
794794

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

796801
// Operator decls
797802
ERROR(ambiguous_operator_decls,none,

include/swift/AST/Module.h

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

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

lib/ClangImporter/ClangImporter.cpp

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

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

18491847
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)