Skip to content

Commit 019bd3c

Browse files
xymusxymus
authored
Merge pull request #67131 from xymus/warn-ioi-without-lib-evolution
[Sema] Warn on `@_implementationOnly` imports from a module without library-evolution
2 parents e790b2d + 635ddf5 commit 019bd3c

15 files changed

+111
-44
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: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -781,26 +781,37 @@ void UnboundImport::validateInterfaceWithPackageName(ModuleDecl *topLevelModule,
781781

782782
void UnboundImport::validateResilience(NullablePtr<ModuleDecl> topLevelModule,
783783
SourceFile &SF) {
784-
if (import.options.contains(ImportFlags::ImplementationOnly) ||
785-
import.accessLevel < AccessLevel::Public)
786-
return;
784+
ASTContext &ctx = SF.getASTContext();
787785

788786
// Per getTopLevelModule(), we'll only get nullptr here for non-Swift modules,
789787
// so these two really mean the same thing.
790788
if (!topLevelModule || topLevelModule.get()->isNonSwiftModule())
791789
return;
792790

793-
ASTContext &ctx = SF.getASTContext();
794-
795791
// If the module we're validating is the builtin one, then just return because
796792
// this module is essentially a header only import and does not concern
797793
// itself with resiliency. This can occur when one has passed
798794
// '-enable-builtin-module' and is explicitly importing the Builtin module in
799795
// their sources.
800-
if (topLevelModule.get() == ctx.TheBuiltinModule) {
796+
if (topLevelModule.get() == ctx.TheBuiltinModule)
801797
return;
798+
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());
802809
}
803810

811+
if (import.options.contains(ImportFlags::ImplementationOnly) ||
812+
import.accessLevel < AccessLevel::Public)
813+
return;
814+
804815
if (!SF.getParentModule()->isResilient() ||
805816
topLevelModule.get()->isResilient())
806817
return;

‎test/Concurrency/preconcurrency_implementationonly_override.swift

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
// RUN: %empty-directory(%t)
2-
// RUN: %target-swift-frontend -emit-module -emit-module-path %t/ImplementationOnlyDefs.swiftmodule -module-name ImplementationOnlyDefs %S/Inputs/ImplementationOnlyDefs.swift
3-
// RUN: %target-typecheck-verify-swift -disable-availability-checking -I %t
2+
// RUN: %target-swift-frontend -emit-module -emit-module-path %t/ImplementationOnlyDefs.swiftmodule -module-name ImplementationOnlyDefs %S/Inputs/ImplementationOnlyDefs.swift \
3+
// RUN: -enable-library-evolution -swift-version 5
4+
// RUN: %target-typecheck-verify-swift -disable-availability-checking -I %t \
5+
// RUN: -enable-library-evolution -swift-version 5
46

57
// REQUIRES: concurrency
68

‎test/SPI/implementation_only_spi_import_exposability.swift

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
/// @_implementationOnly imported decls (SPI or not) should not be exposed in SPI.
22

33
// RUN: %empty-directory(%t)
4-
// RUN: %target-swift-frontend -emit-module -DLIB %s -module-name Lib -emit-module-path %t/Lib.swiftmodule
5-
// RUN: %target-typecheck-verify-swift -DCLIENT -I %t
4+
// RUN: %target-swift-frontend -emit-module -DLIB %s -module-name Lib -emit-module-path %t/Lib.swiftmodule \
5+
// RUN: -swift-version 5 -enable-library-evolution
6+
// RUN: %target-typecheck-verify-swift -DCLIENT -I %t \
7+
// RUN: -swift-version 5 -enable-library-evolution
68

79
#if LIB
810

‎test/SPI/report-ioi-in-spi.swift

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,22 +2,21 @@
22
// RUN: split-file %s %t
33

44
// RUN: %target-swift-frontend -emit-module %t/Lib.swift -I %t \
5+
// RUN: -enable-library-evolution \
56
// RUN: -module-name Lib -emit-module-path %t/Lib.swiftmodule \
67
// RUN: -swift-version 5
78

89
/// Use of IOI types in SPI signatures is an error with -experimental-spi-only-imports
910
// RUN: %target-swift-frontend -emit-module %t/ClientSPIOnlyMode.swift -I %t \
11+
// RUN: -enable-library-evolution \
1012
// RUN: -swift-version 5 -verify \
1113
// RUN: -experimental-spi-only-imports
1214

1315
/// Use of IOI types in SPI signatures is a warning without -experimental-spi-only-imports
1416
// RUN: %target-swift-frontend -emit-module %t/ClientDefaultMode.swift -I %t \
17+
// RUN: -enable-library-evolution \
1518
// RUN: -swift-version 5 -verify
1619

17-
/// This is a warning in swiftinterfaces
18-
// R UN: %target-swift-typecheck-module-from-interface(%t/Client.private.swiftinterface) \
19-
// R UN: -I %t -module-name Client
20-
2120
//--- Lib.swift
2221

2322
public struct IOIStruct {
@@ -49,12 +48,3 @@ public struct IOIStruct {
4948
return IOIStruct() // expected-error {{struct 'IOIStruct' cannot be used in an '@inlinable' function because 'Lib' was imported implementation-only}}
5049
// expected-error @-1 {{initializer 'init()' cannot be used in an '@inlinable' function because 'Lib' was imported implementation-only}}
5150
}
52-
53-
//--- Client.private.swiftinterface
54-
55-
// swift-interface-format-version: 1.0
56-
// swift-compiler-version: Swift version 5.8-dev effective-4.1.50
57-
// swift-module-flags: -swift-version 4 -module-name Client
58-
@_implementationOnly import Lib
59-
60-
@_spi(X) public func spiClient() -> IOIStruct { fatalError() }

‎test/SPI/spi-only-import-conflicting.swift

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,21 +10,29 @@
1010

1111
/// Build clients.
1212
// RUN: %target-swift-frontend -typecheck %t/SPIOnly_Default.swift -I %t -verify \
13+
// RUN: -swift-version 5 -enable-library-evolution \
1314
// RUN: -experimental-spi-only-imports -verify
1415
// RUN: %target-swift-frontend -typecheck %t/Default_SPIOnly.swift -I %t -verify \
16+
// RUN: -swift-version 5 -enable-library-evolution \
1517
// RUN: -experimental-spi-only-imports -verify
1618
// RUN: %target-swift-frontend -typecheck %t/SPIOnly_Exported.swift -I %t -verify \
19+
// RUN: -swift-version 5 -enable-library-evolution \
1720
// RUN: -experimental-spi-only-imports -verify
1821
// RUN: %target-swift-frontend -typecheck %t/Exported_SPIOnly.swift -I %t -verify \
22+
// RUN: -swift-version 5 -enable-library-evolution \
1923
// RUN: -experimental-spi-only-imports -verify
2024
// RUN: %target-swift-frontend -typecheck %t/SPIOnly_IOI.swift -I %t -verify \
25+
// RUN: -swift-version 5 -enable-library-evolution \
2126
// RUN: -experimental-spi-only-imports -verify
2227
// RUN: %target-swift-frontend -typecheck %t/SPIOnly_IOI_Exported_Default.swift -I %t -verify \
28+
// RUN: -swift-version 5 -enable-library-evolution \
2329
// RUN: -experimental-spi-only-imports -verify
2430
// RUN: %target-swift-frontend -typecheck -primary-file %t/SPIOnly_Default_FileA.swift \
31+
// RUN: -swift-version 5 -enable-library-evolution \
2532
// RUN: %t/SPIOnly_Default_FileB.swift -I %t -verify \
2633
// RUN: -experimental-spi-only-imports -verify
2734
// RUN: %target-swift-frontend -typecheck -primary-file %t/IOI_Default_FileA.swift \
35+
// RUN: -swift-version 5 -enable-library-evolution \
2836
// RUN: %t/IOI_Default_FileB.swift -I %t -verify \
2937
// RUN: -experimental-spi-only-imports -verify
3038

‎test/Sema/implementation-only-import-from-non-resilient.swift

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

‎test/Sema/implementation-only-import-in-decls.swift

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
// RUN: %empty-directory(%t)
2-
// RUN: %target-swift-frontend -emit-module -o %t/NormalLibrary.swiftmodule %S/Inputs/implementation-only-import-in-decls-public-helper.swift
3-
// RUN: %target-swift-frontend -emit-module -o %t/BADLibrary.swiftmodule %S/Inputs/implementation-only-import-in-decls-helper.swift -I %t
2+
// RUN: %target-swift-frontend -emit-module -o %t/NormalLibrary.swiftmodule %S/Inputs/implementation-only-import-in-decls-public-helper.swift \
3+
// RUN: -enable-library-evolution -swift-version 5
4+
// RUN: %target-swift-frontend -emit-module -o %t/BADLibrary.swiftmodule %S/Inputs/implementation-only-import-in-decls-helper.swift -I %t \
5+
// RUN: -enable-library-evolution -swift-version 5
46

5-
// RUN: %target-typecheck-verify-swift -I %t
7+
// RUN: %target-typecheck-verify-swift -I %t \
8+
// RUN: -enable-library-evolution -swift-version 5
69

710
import NormalLibrary
811
@_implementationOnly import BADLibrary

‎test/Sema/implementation-only-import-inlinable-conformances.swift

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
// RUN: %empty-directory(%t)
2-
// RUN: %target-swift-frontend -emit-module -o %t/NormalLibrary.swiftmodule %S/Inputs/implementation-only-import-in-decls-public-helper.swift
3-
// RUN: %target-swift-frontend -emit-module -o %t/BADLibrary.swiftmodule %S/Inputs/implementation-only-import-in-decls-helper.swift -I %t
2+
// RUN: %target-swift-frontend -emit-module -o %t/NormalLibrary.swiftmodule %S/Inputs/implementation-only-import-in-decls-public-helper.swift \
3+
// RUN: -enable-library-evolution -swift-version 5
4+
// RUN: %target-swift-frontend -emit-module -o %t/BADLibrary.swiftmodule %S/Inputs/implementation-only-import-in-decls-helper.swift -I %t \
5+
// RUN: -enable-library-evolution -swift-version 5
46

5-
// RUN: %target-typecheck-verify-swift -I %t
7+
// RUN: %target-typecheck-verify-swift -I %t -enable-library-evolution -swift-version 5
68

79
@_implementationOnly import BADLibrary
810
import NormalLibrary

‎test/Sema/implementation-only-import-inlinable-indirect.swift

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
// RUN: %empty-directory(%t)
2-
// RUN: %target-swift-frontend -emit-module -o %t/indirects.swiftmodule %S/Inputs/implementation-only-imports/indirects.swift
3-
// RUN: %target-swift-frontend -emit-module -o %t/directs.swiftmodule -I %t %S/Inputs/implementation-only-imports/directs.swift
2+
// RUN: %target-swift-frontend -emit-module -o %t/indirects.swiftmodule %S/Inputs/implementation-only-imports/indirects.swift \
3+
// RUN: -enable-library-evolution -swift-version 5
4+
// RUN: %target-swift-frontend -emit-module -o %t/directs.swiftmodule -I %t %S/Inputs/implementation-only-imports/directs.swift \
5+
// RUN: -enable-library-evolution -swift-version 5
46

5-
// RUN: %target-swift-frontend -typecheck -verify %s -I %t
7+
// RUN: %target-swift-frontend -typecheck -verify %s -I %t \
8+
// RUN: -enable-library-evolution -swift-version 5
69

710
@_implementationOnly import directs
811

‎test/Sema/implementation-only-import-inlinable-multifile.swift

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
// RUN: %empty-directory(%t)
2-
// RUN: %target-swift-frontend -emit-module -o %t/indirects.swiftmodule %S/Inputs/implementation-only-imports/indirects.swift
3-
// RUN: %target-swift-frontend -emit-module -o %t/directs.swiftmodule -I %t %S/Inputs/implementation-only-imports/directs.swift
2+
// RUN: %target-swift-frontend -emit-module -o %t/indirects.swiftmodule %S/Inputs/implementation-only-imports/indirects.swift \
3+
// RUN: -enable-library-evolution -swift-version 5
4+
// RUN: %target-swift-frontend -emit-module -o %t/directs.swiftmodule -I %t %S/Inputs/implementation-only-imports/directs.swift \
5+
// RUN: -enable-library-evolution -swift-version 5
46

5-
// RUN: %target-swift-frontend -typecheck -verify -primary-file %s %S/Inputs/implementation-only-imports/secondary_file.swift -I %t
7+
// RUN: %target-swift-frontend -typecheck -verify -primary-file %s %S/Inputs/implementation-only-imports/secondary_file.swift -I %t \
8+
// RUN: -enable-library-evolution -swift-version 5
69

710
@_implementationOnly import directs
811
// 'indirects' is imported for re-export in a secondary file

‎test/Sema/implementation-only-import-inlinable.swift

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
// RUN: %empty-directory(%t)
2-
// RUN: %target-swift-frontend -emit-module -o %t/indirects.swiftmodule %S/Inputs/implementation-only-imports/indirects.swift
3-
// RUN: %target-swift-frontend -emit-module -o %t/directs.swiftmodule -I %t %S/Inputs/implementation-only-imports/directs.swift
2+
// RUN: %target-swift-frontend -emit-module -o %t/indirects.swiftmodule %S/Inputs/implementation-only-imports/indirects.swift \
3+
// RUN: -enable-library-evolution -swift-version 5
4+
// RUN: %target-swift-frontend -emit-module -o %t/directs.swiftmodule -I %t %S/Inputs/implementation-only-imports/directs.swift \
5+
// RUN: -enable-library-evolution -swift-version 5
46

5-
// RUN: %target-swift-frontend -typecheck -verify %s -I %t
7+
// RUN: %target-swift-frontend -typecheck -verify %s -I %t \
8+
// RUN: -enable-library-evolution -swift-version 5
69

710
@_implementationOnly import directs
811
import indirects

‎test/Sema/missing-import-inlinable-code.swift

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,24 @@
33
// RUN: %empty-directory(%t)
44
// RUN: %{python} %utils/split_file.py -o %t %s
55

6-
// RUN: %target-swift-frontend -emit-module %t/empty.swift -module-name empty -o %t/empty.swiftmodule
7-
// RUN: %target-swift-frontend -emit-module %t/libA.swift -module-name libA -o %t/libA.swiftmodule
8-
// RUN: %target-swift-frontend -emit-module %t/libB.swift -module-name libB -o %t/libB.swiftmodule -I %t
6+
// RUN: %target-swift-frontend -emit-module %t/empty.swift -module-name empty -o %t/empty.swiftmodule \
7+
// RUN: -enable-library-evolution
8+
// RUN: %target-swift-frontend -emit-module %t/libA.swift -module-name libA -o %t/libA.swiftmodule \
9+
// RUN: -enable-library-evolution
10+
// RUN: %target-swift-frontend -emit-module %t/libB.swift -module-name libB -o %t/libB.swiftmodule -I %t \
11+
// RUN: -enable-library-evolution
912

1013
/// In pre-Swift 6, this is a warning where there's no implementation-only import present.
11-
// RUN: %target-swift-frontend -emit-module %t/clientFileA-Swift5.swift %t/clientFileB.swift -module-name client -o %t/client.swiftmodule -I %t -verify
14+
// RUN: %target-swift-frontend -emit-module %t/clientFileA-Swift5.swift %t/clientFileB.swift -module-name client -o %t/client.swiftmodule -I %t -verify \
15+
// RUN: -enable-library-evolution
1216

1317
/// In pre-Swift 6, this remains an error when there's an implementation-only import present.
14-
// RUN: %target-swift-frontend -emit-module %t/clientFileA-OldCheck.swift %t/clientFileB.swift -module-name client -o %t/client.swiftmodule -I %t -verify
18+
// RUN: %target-swift-frontend -emit-module %t/clientFileA-OldCheck.swift %t/clientFileB.swift -module-name client -o %t/client.swiftmodule -I %t -verify \
19+
// RUN: -enable-library-evolution
1520

1621
/// In Swift 6, it's an error.
17-
// RUN: %target-swift-frontend -emit-module %t/clientFileA-Swift6.swift %t/clientFileB.swift -module-name client -o %t/client.swiftmodule -I %t -verify -swift-version 6
22+
// RUN: %target-swift-frontend -emit-module %t/clientFileA-Swift6.swift %t/clientFileB.swift -module-name client -o %t/client.swiftmodule -I %t -verify -swift-version 6 \
23+
// RUN: -enable-library-evolution
1824

1925
/// The swiftinterface is broken by the missing import without the workaround.
2026
// RUN: %target-swift-emit-module-interface(%t/ClientBroken.swiftinterface) %t/clientFileA-Swift5.swift %t/clientFileB.swift -I %t -disable-print-missing-imports-in-module-interface

‎test/Sema/restricted-imports-priorities.swift

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,22 @@
1616
// RUN: -swift-version 5 -enable-library-evolution -I %t
1717

1818
// RUN: %target-swift-frontend -typecheck %t/TwoIOI.swift -I %t -verify \
19+
// RUN: -swift-version 5 -enable-library-evolution \
1920
// RUN: -experimental-spi-only-imports -verify
2021
// RUN: %target-swift-frontend -typecheck %t/SPIOnlyAndIOI1.swift -I %t -verify \
22+
// RUN: -swift-version 5 -enable-library-evolution \
2123
// RUN: -experimental-spi-only-imports -verify
2224
// RUN: %target-swift-frontend -typecheck %t/SPIOnlyAndIOI2.swift -I %t -verify \
25+
// RUN: -swift-version 5 -enable-library-evolution \
2326
// RUN: -experimental-spi-only-imports -verify
2427
// RUN: %target-swift-frontend -typecheck %t/TwoSPIOnly.swift -I %t -verify \
28+
// RUN: -swift-version 5 -enable-library-evolution \
2529
// RUN: -experimental-spi-only-imports -verify
2630
// RUN: %target-swift-frontend -typecheck %t/OneSPIOnly1.swift -I %t -verify \
31+
// RUN: -swift-version 5 -enable-library-evolution \
2732
// RUN: -experimental-spi-only-imports -verify
2833
// RUN: %target-swift-frontend -typecheck %t/OneSPIOnly2.swift -I %t -verify \
34+
// RUN: -swift-version 5 -enable-library-evolution \
2935
// RUN: -experimental-spi-only-imports -verify
3036

3137
/// Setup 2 indirect imports of LibA, allowing the same LibA to be imported

‎test/decl/import/import.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,3 +74,4 @@ import func français.phoûx
7474
import main // expected-warning {{file 'import.swift' is part of module 'main'; ignoring import}}
7575

7676
@_exported @_implementationOnly import empty // expected-error {{module 'empty' cannot be both exported and implementation-only}} {{12-33=}}
77+
// expected-warning @-1 {{using '@_implementationOnly' without enabling library evolution for 'main' may lead to instability during execution}}

0 commit comments

Comments
 (0)