Skip to content

Commit d71bd2d

Browse files
xymusxymus
authored
Merge pull request #76267 from xymus/warn-on-ioi-clang-targets
Sema: Warn on all non-resilient uses of `@_implementationOnly import`, even for clang targets
2 parents 2fd182a + 6c3c108 commit d71bd2d

7 files changed

+57
-30
lines changed

‎lib/Sema/ImportResolution.cpp‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -792,18 +792,15 @@ void UnboundImport::validateInterfaceWithPackageName(ModuleDecl *topLevelModule,
792792

793793
void UnboundImport::validateResilience(NullablePtr<ModuleDecl> topLevelModule,
794794
SourceFile &SF) {
795-
ASTContext &ctx = SF.getASTContext();
796-
797-
// Per getTopLevelModule(), we'll only get nullptr here for non-Swift modules,
798-
// so these two really mean the same thing.
799-
if (!topLevelModule || topLevelModule.get()->isNonSwiftModule())
795+
if (!topLevelModule)
800796
return;
801797

802798
// If the module we're validating is the builtin one, then just return because
803799
// this module is essentially a header only import and does not concern
804800
// itself with resiliency. This can occur when one has passed
805801
// '-enable-builtin-module' and is explicitly importing the Builtin module in
806802
// their sources.
803+
ASTContext &ctx = SF.getASTContext();
807804
if (topLevelModule.get() == ctx.TheBuiltinModule)
808805
return;
809806

@@ -821,11 +818,13 @@ void UnboundImport::validateResilience(NullablePtr<ModuleDecl> topLevelModule,
821818
import.implementationOnlyRange.isValid()) {
822819
if (SF.getParentModule()->isResilient()) {
823820
// Encourage replacing `@_implementationOnly` with `internal import`.
824-
auto inFlight =
825-
ctx.Diags.diagnose(import.importLoc,
826-
diag::implementation_only_deprecated);
827-
inFlight.fixItReplace(import.implementationOnlyRange, "internal");
828-
} else if ( // Non-resilient
821+
if (!topLevelModule.get()->isNonSwiftModule()) {
822+
auto inFlight =
823+
ctx.Diags.diagnose(import.importLoc,
824+
diag::implementation_only_deprecated);
825+
inFlight.fixItReplace(import.implementationOnlyRange, "internal");
826+
}
827+
} else if ( // Non-resilient client
829828
!(((targetName.str() == "CCryptoBoringSSL" ||
830829
targetName.str() == "CCryptoBoringSSLShims") &&
831830
(importerName.str() == "Crypto" ||
@@ -841,7 +840,8 @@ void UnboundImport::validateResilience(NullablePtr<ModuleDecl> topLevelModule,
841840
}
842841

843842
// Report public imports of non-resilient modules from a resilient module.
844-
if (import.options.contains(ImportFlags::ImplementationOnly) ||
843+
if (topLevelModule.get()->isNonSwiftModule() ||
844+
import.options.contains(ImportFlags::ImplementationOnly) ||
845845
import.accessLevel < AccessLevel::Public)
846846
return;
847847

‎test/ClangImporter/objc_init_redundant.swift‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk -I %S/Inputs/custom-modules -F %S/Inputs/frameworks) -import-underlying-module -import-objc-header %S/Inputs/objc_init_redundant_bridging.h -emit-sil %s -verify
2-
// RUN: not %target-swift-frontend(mock-sdk: %clang-importer-sdk -I %S/Inputs/custom-modules -F %S/Inputs/frameworks) -import-underlying-module -import-objc-header %S/Inputs/objc_init_redundant_bridging.h -emit-sil %s > %t.log 2>&1
1+
// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk -I %S/Inputs/custom-modules -F %S/Inputs/frameworks) -import-underlying-module -import-objc-header %S/Inputs/objc_init_redundant_bridging.h -emit-sil %s -verify -swift-version 5 -enable-library-evolution
2+
// RUN: not %target-swift-frontend(mock-sdk: %clang-importer-sdk -I %S/Inputs/custom-modules -F %S/Inputs/frameworks) -import-underlying-module -import-objc-header %S/Inputs/objc_init_redundant_bridging.h -emit-sil %s -swift-version 5 -enable-library-evolution > %t.log 2>&1
33
// RUN: %FileCheck %s < %t.log
44

55
// REQUIRES: objc_interop

‎test/ModuleInterface/imports.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// RUN: %empty-directory(%t)
22
// RUN: %target-swift-frontend -disable-implicit-concurrency-module-import -disable-implicit-string-processing-module-import -emit-module -o %t/empty.swiftmodule %S/../Inputs/empty.swift
33
// RUN: %target-swift-frontend -disable-implicit-concurrency-module-import -disable-implicit-string-processing-module-import -emit-module -o %t/emptyButWithLibraryEvolution.swiftmodule %S/../Inputs/empty.swift -enable-library-evolution
4-
// RUN: %target-swift-emit-module-interface(%t.swiftinterface) %s %S/Inputs/imports-other.swift -disable-implicit-concurrency-module-import -disable-implicit-string-processing-module-import -I %S/Inputs/imports-clang-modules/ -I %t -verify
4+
// RUN: %target-swift-emit-module-interface(%t.swiftinterface) %s %S/Inputs/imports-other.swift -disable-implicit-concurrency-module-import -disable-implicit-string-processing-module-import -I %S/Inputs/imports-clang-modules/ -I %t -verify -swift-version 5 -enable-library-evolution
55
// RUN: %target-swift-typecheck-module-from-interface(%t.swiftinterface) -I %S/Inputs/imports-clang-modules/ -I %t
66
// RUN: %FileCheck -implicit-check-not BAD %s < %t.swiftinterface
77

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

Lines changed: 36 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
// RUN: %empty-directory(%t)
2-
// RUN: split-file %s %t
2+
// RUN: split-file %s %t --leading-lines
33

44
/// Build 2 libs.
5-
// RUN: %target-swift-frontend -emit-module %t/empty.swift -o %t/A.swiftmodule \
5+
// RUN: %target-swift-frontend -emit-module %t/empty.swift -o %t/SwiftModuleA.swiftmodule \
66
// RUN: -enable-library-evolution -swift-version 5
7-
// RUN: %target-swift-frontend -emit-module %t/empty.swift -o %t/B.swiftmodule \
7+
// RUN: %target-swift-frontend -emit-module %t/empty.swift -o %t/SwiftModuleB.swiftmodule \
88
// RUN: -enable-library-evolution -swift-version 5
99

1010
/// Build a client with and without library-evolution.
@@ -19,18 +19,45 @@
1919
// RUN: %target-swift-frontend -typecheck %t/Crypto.swift -I %t -verify \
2020
// RUN: -module-name Crypto
2121

22+
//--- module.modulemap
23+
module ClangModuleA {
24+
header "ClangModuleA.h"
25+
26+
module Submodule {
27+
header "ClangSubmodule.h"
28+
}
29+
}
30+
31+
module ClangModuleB {
32+
header "ClangModuleB.h"
33+
}
34+
35+
//--- ClangModuleA.h
36+
//--- ClangSubmodule.h
37+
//--- ClangModuleB.h
38+
2239
//--- empty.swift
2340

2441
//--- client-non-resilient.swift
25-
@_implementationOnly import A // expected-warning {{using '@_implementationOnly' without enabling library evolution for 'main' may lead to instability during execution}}
26-
import B
42+
@_implementationOnly import SwiftModuleA // expected-warning {{using '@_implementationOnly' without enabling library evolution for 'main' may lead to instability during execution}}
43+
@_implementationOnly import SwiftModuleA // expected-warning {{using '@_implementationOnly' without enabling library evolution for 'main' may lead to instability during execution}}
44+
import SwiftModuleB
45+
46+
@_implementationOnly import ClangModuleA // expected-warning {{using '@_implementationOnly' without enabling library evolution for 'main' may lead to instability during execution}}
47+
@_implementationOnly import ClangModuleA.Submodule // expected-warning {{using '@_implementationOnly' without enabling library evolution for 'main' may lead to instability during execution}}
48+
import ClangModuleB
2749

2850
//--- client-resilient.swift
29-
@_implementationOnly import A
51+
@_implementationOnly import SwiftModuleA
3052
// expected-warning @-1 {{'@_implementationOnly' is deprecated, use 'internal import' instead}}
31-
import B
53+
import SwiftModuleB
54+
55+
@_implementationOnly import ClangModuleA
56+
@_implementationOnly import ClangModuleA.Submodule
57+
import ClangModuleB
3258

3359
//--- Crypto.swift
34-
@_implementationOnly import A // expected-warning {{using '@_implementationOnly' without enabling library evolution for 'Crypto' may lead to instability during execution}}
35-
import B
60+
@_implementationOnly import SwiftModuleA // expected-warning {{using '@_implementationOnly' without enabling library evolution for 'Crypto' may lead to instability during execution}}
61+
import SwiftModuleB
3662
@_implementationOnly import CCryptoBoringSSL
63+
import ClangModuleB

‎test/decl/ext/objc_implementation_class_extension.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-typecheck-verify-swift -verify-ignore-unknown -import-underlying-module -Xcc -fmodule-map-file=%S/Inputs/objc_implementation_class_extension.modulemap -target %target-stable-abi-triple
1+
// RUN: %target-typecheck-verify-swift -verify-ignore-unknown -import-underlying-module -Xcc -fmodule-map-file=%S/Inputs/objc_implementation_class_extension.modulemap -target %target-stable-abi-triple -swift-version 5 -enable-library-evolution
22
// REQUIRES: objc_interop
33

44
@_implementationOnly import objc_implementation_class_extension_internal

‎test/decl/ext/objc_implementation_impl_only.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-typecheck-verify-swift -Xcc -fmodule-map-file=%S/Inputs/objc_implementation_private.modulemap -target %target-stable-abi-triple
1+
// RUN: %target-typecheck-verify-swift -Xcc -fmodule-map-file=%S/Inputs/objc_implementation_private.modulemap -target %target-stable-abi-triple -swift-version 5 -enable-library-evolution
22
// REQUIRES: objc_interop
33

44
@_implementationOnly import objc_implementation_internal

‎test/decl/import/inconsistent-implementation-only.swift‎

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

44
// Check that the diagnostics are produced regardless of what primary file we're using.
5-
// RUN: %target-swift-frontend -typecheck -primary-file %t/1.swift %t/2.swift %t/3.swift -I %S/Inputs/inconsistent-implementation-only/ -verify
6-
// RUN: %target-swift-frontend -typecheck %t/1.swift -primary-file %t/2.swift %t/3.swift -I %S/Inputs/inconsistent-implementation-only/ -verify
7-
// RUN: %target-swift-frontend -typecheck %t/1.swift %t/2.swift -primary-file %t/3.swift -I %S/Inputs/inconsistent-implementation-only/ -verify
8-
// RUN: %target-swift-frontend -typecheck -primary-file %t/1.swift -primary-file %t/2.swift -primary-file %t/3.swift -I %S/Inputs/inconsistent-implementation-only/ -verify
9-
// RUN: %target-swift-frontend -typecheck %t/1.swift %t/2.swift %t/3.swift -I %S/Inputs/inconsistent-implementation-only/ -verify
5+
// RUN: %target-swift-frontend -typecheck -primary-file %t/1.swift %t/2.swift %t/3.swift -I %S/Inputs/inconsistent-implementation-only/ -verify -swift-version 5 -enable-library-evolution
6+
// RUN: %target-swift-frontend -typecheck %t/1.swift -primary-file %t/2.swift %t/3.swift -I %S/Inputs/inconsistent-implementation-only/ -verify -swift-version 5 -enable-library-evolution
7+
// RUN: %target-swift-frontend -typecheck %t/1.swift %t/2.swift -primary-file %t/3.swift -I %S/Inputs/inconsistent-implementation-only/ -verify -swift-version 5 -enable-library-evolution
8+
// RUN: %target-swift-frontend -typecheck -primary-file %t/1.swift -primary-file %t/2.swift -primary-file %t/3.swift -I %S/Inputs/inconsistent-implementation-only/ -verify -swift-version 5 -enable-library-evolution
9+
// RUN: %target-swift-frontend -typecheck %t/1.swift %t/2.swift %t/3.swift -I %S/Inputs/inconsistent-implementation-only/ -verify -swift-version 5 -enable-library-evolution
1010

1111
//--- 1.swift
1212

0 commit comments

Comments
 (0)