Skip to content

Commit 558f8d1

Browse files
committed
Sema: Warn on all non-resilient uses of @_implementationOnly, even for clang targets
The warnings about `using '@_implementationOnly' without enabling library evolution for 'client' may lead to instability during execution` and `@_implementationOnly' is deprecated, use 'internal import' instead` were wrongly restricted to only Swift import targets. Make sure they are raised for clang module targets as well. rdar://135233043
1 parent 1f93fb3 commit 558f8d1

File tree

2 files changed

+42
-15
lines changed

2 files changed

+42
-15
lines changed

lib/Sema/ImportResolution.cpp

Lines changed: 4 additions & 6 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

@@ -841,7 +838,8 @@ void UnboundImport::validateResilience(NullablePtr<ModuleDecl> topLevelModule,
841838
}
842839

843840
// Report public imports of non-resilient modules from a resilient module.
844-
if (import.options.contains(ImportFlags::ImplementationOnly) ||
841+
if (topLevelModule.get()->isNonSwiftModule() ||
842+
import.options.contains(ImportFlags::ImplementationOnly) ||
845843
import.accessLevel < AccessLevel::Public)
846844
return;
847845

Lines changed: 38 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,47 @@
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
52+
// expected-warning @-1 {{'@_implementationOnly' is deprecated, use 'internal import' instead}}
53+
import SwiftModuleB
54+
55+
@_implementationOnly import ClangModuleA
56+
// expected-warning @-1 {{'@_implementationOnly' is deprecated, use 'internal import' instead}}
57+
@_implementationOnly import ClangModuleA.Submodule
3058
// expected-warning @-1 {{'@_implementationOnly' is deprecated, use 'internal import' instead}}
31-
import B
59+
import ClangModuleB
3260

3361
//--- 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
62+
@_implementationOnly import SwiftModuleA // expected-warning {{using '@_implementationOnly' without enabling library evolution for 'Crypto' may lead to instability during execution}}
63+
import SwiftModuleB
3664
@_implementationOnly import CCryptoBoringSSL
65+
import ClangModuleB

0 commit comments

Comments
 (0)