Skip to content

Commit fabb345

Browse files
committed
[Sema] Warn on uses of @_implementationOnly imports without library-evolution
@_implementationOnly was designed for use from resilient modules only, using it from non-resilient modules in unsupported. This change adds a warning about it. If anyone hits this warning, they should either enable library-evolution or consider adopting the new `internal import` when it is available as it handles this scenario properly. What leads to a crash: an @_implementationOnly import fully hides the dependency from indirect clients. This can lead to the compiler being unaware of some internal details of a non-resilient module when building a client against it. In turn this may lead to run time crashes from miscompilation. In general one could still use @_implementationOnly in a non-resilient modules as long as it's referenced only from function bodies. However, references from even non-public properties does lead to important memory layout information being hidden from clients of the module. rdar://78129903
1 parent 671cc80 commit fabb345

File tree

3 files changed

+39
-0
lines changed

3 files changed

+39
-0
lines changed

include/swift/AST/DiagnosticsSema.def

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1098,6 +1098,10 @@ ERROR(module_not_compiled_with_library_evolution,none,
10981098
"module %0 was not compiled with library evolution support; "
10991099
"using it means binary compatibility for %1 can't be guaranteed",
11001100
(Identifier, Identifier))
1101+
WARNING(implementation_only_requires_library_evolution,none,
1102+
"using '@_implementationOnly' without enabling library evolution "
1103+
"for %0 may lead to instability during execution",
1104+
(Identifier))
11011105

11021106
ERROR(module_allowable_client_violation,none,
11031107
"module %0 doesn't allow importation from module %1",

lib/Sema/ImportResolution.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -796,6 +796,18 @@ void UnboundImport::validateResilience(NullablePtr<ModuleDecl> topLevelModule,
796796
if (topLevelModule.get() == ctx.TheBuiltinModule)
797797
return;
798798

799+
// @_implementationOnly is only supported when used from modules built with
800+
// library-evolution. Otherwise it can lead to runtime crashes from a lack
801+
// of memory layout information when building clients unaware of the
802+
// dependency. The missing information is provided at run time by resilient
803+
// modules.
804+
if (import.options.contains(ImportFlags::ImplementationOnly) &&
805+
!SF.getParentModule()->isResilient() && topLevelModule) {
806+
ctx.Diags.diagnose(import.importLoc,
807+
diag::implementation_only_requires_library_evolution,
808+
SF.getParentModule()->getName());
809+
}
810+
799811
if (import.options.contains(ImportFlags::ImplementationOnly) ||
800812
import.accessLevel < AccessLevel::Public)
801813
return;
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: split-file %s %t
3+
4+
/// Build 2 libs.
5+
// RUN: %target-swift-frontend -emit-module %t/empty.swift -o %t/A.swiftmodule \
6+
// RUN: -enable-library-evolution -swift-version 5
7+
// RUN: %target-swift-frontend -emit-module %t/empty.swift -o %t/B.swiftmodule \
8+
// RUN: -enable-library-evolution -swift-version 5
9+
10+
/// Build a client with and without library-evolution.
11+
// RUN: %target-swift-frontend -typecheck %t/client-non-resilient.swift -I %t -verify
12+
// RUN: %target-swift-frontend -typecheck %t/client-resilient.swift -I %t -verify \
13+
// RUN: -enable-library-evolution -swift-version 5
14+
15+
//--- empty.swift
16+
17+
//--- client-non-resilient.swift
18+
@_implementationOnly import A // expected-warning {{using '@_implementationOnly' without enabling library evolution for 'main' may lead to instability during execution}}
19+
import B
20+
21+
//--- client-resilient.swift
22+
@_implementationOnly import A
23+
import B

0 commit comments

Comments
 (0)