Skip to content

Commit e97db1c

Browse files
xymusxymus
authored
Merge pull request #79487 from xymus/limit-nserror-optimization-to-import
SILGen: Ensure Foundation is publicly imported before referencing `NSError` in code gen
2 parents c08a1ee + 76f026d commit e97db1c

File tree

5 files changed

+255
-1
lines changed

5 files changed

+255
-1
lines changed

‎lib/SILGen/SILGenConvert.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -656,9 +656,10 @@ ManagedValue SILGenFunction::emitExistentialErasure(
656656
// If we're erasing to the 'Error' type, we might be able to get an NSError
657657
// representation more efficiently.
658658
auto &ctx = getASTContext();
659+
auto *nsErrorDecl = ctx.getNSErrorDecl();
659660
if (ctx.LangOpts.EnableObjCInterop && conformances.size() == 1 &&
660661
conformances[0].getRequirement() == ctx.getErrorDecl() &&
661-
ctx.getNSErrorDecl()) {
662+
nsErrorDecl && referenceAllowed(nsErrorDecl)) {
662663
// If the concrete type is NSError or a subclass thereof, just erase it
663664
// directly.
664665
auto nsErrorType = ctx.getNSErrorType()->getCanonicalType();

‎lib/SILGen/SILGenFunction.cpp

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include "swift/AST/FileUnit.h"
3131
#include "swift/AST/GenericEnvironment.h"
3232
#include "swift/AST/Initializer.h"
33+
#include "swift/AST/ImportCache.h"
3334
#include "swift/AST/ParameterList.h"
3435
#include "swift/AST/PropertyWrappers.h"
3536
#include "swift/AST/SourceFile.h"
@@ -210,6 +211,35 @@ DeclName SILGenModule::getMagicFunctionName(SILDeclRef ref) {
210211
llvm_unreachable("Unhandled SILDeclRefKind in switch.");
211212
}
212213

214+
bool SILGenFunction::referenceAllowed(ValueDecl *decl) {
215+
// Use in any non-fragile functions.
216+
if (FunctionDC->getResilienceExpansion() == ResilienceExpansion::Maximal)
217+
return true;
218+
219+
// Allow same-module references.
220+
auto *targetMod = decl->getDeclContext()->getParentModule();
221+
auto *thisMod = FunctionDC->getParentModule();
222+
if (thisMod == targetMod)
223+
return true;
224+
225+
ModuleDecl::ImportFilter filter = {
226+
ModuleDecl::ImportFilterKind::Exported,
227+
ModuleDecl::ImportFilterKind::Default,
228+
ModuleDecl::ImportFilterKind::SPIOnly};
229+
if (thisMod->getResilienceStrategy() != ResilienceStrategy::Resilient)
230+
filter |= ModuleDecl::ImportFilterKind::InternalOrBelow;
231+
232+
// Look through public module local imports and their reexports.
233+
llvm::SmallVector<ImportedModule, 8> imports;
234+
thisMod->getImportedModules(imports, filter);
235+
auto &importCache = getASTContext().getImportCache();
236+
for (auto &import : imports) {
237+
if (importCache.isImportedBy(targetMod, import.importedModule))
238+
return true;
239+
}
240+
return false;
241+
}
242+
213243
SILDebugLocation SILGenFunction::getSILDebugLocation(
214244
SILBuilder &B, SILLocation Loc,
215245
std::optional<SILLocation> CurDebugLocOverride, bool ForMetaInstruction) {

‎lib/SILGen/SILGenFunction.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -789,6 +789,15 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
789789
bool isEmittingTopLevelCode() { return IsEmittingTopLevelCode; }
790790
void stopEmittingTopLevelCode() { IsEmittingTopLevelCode = false; }
791791

792+
/// Can the generated code reference \c decl safely?
793+
///
794+
/// Checks that the module defining \c decl is as visible to clients as the
795+
/// code referencing it, preventing an inlinable function to reference an
796+
/// implementation-only dependency and similar. This applies similar checks
797+
/// as the exportability checker does to source code for decls referenced by
798+
/// generated code.
799+
bool referenceAllowed(ValueDecl *decl);
800+
792801
std::optional<SILAccessEnforcement>
793802
getStaticEnforcement(VarDecl *var = nullptr);
794803
std::optional<SILAccessEnforcement>

‎test/SILGen/objc_error_fast_path.swift

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: split-file %s %t --leading-lines
3+
// REQUIRES: objc_interop
4+
// REQUIRES: executable_test
5+
6+
/// Optimization active: public import
7+
// RUN: %target-build-swift-dylib(%t/%target-library-name(Lib)) \
8+
// RUN: %t/optimized.swift -module-name Lib \
9+
// RUN: -emit-module -emit-module-path %t/Lib.swiftmodule \
10+
// RUN: -swift-version 5 -enable-library-evolution -O
11+
// RUN: %target-build-swift %t/main.swift -o %t/main -I%t -lLib -L%t -O
12+
// RUN: %target-codesign %t/main
13+
// RUN: %target-run %t/main | %FileCheck %s
14+
15+
/// Ensure the client has the optimization we're testing here.
16+
// RUN: %target-swift-frontend -typecheck -emit-silgen %t/Lib.swiftmodule > %t/Lib.sil
17+
// RUN: %FileCheck --check-prefix CHECK-OPTIMIZED --input-file %t/Lib.sil %s
18+
// CHECK-OPTIMIZED: _getErrorEmbeddedNSError
19+
20+
/// No optimization: internal import from resilient module
21+
// RUN: %target-build-swift-dylib(%t/%target-library-name(Lib)) \
22+
// RUN: %t/non-optimized.swift -module-name Lib \
23+
// RUN: -emit-module -emit-module-path %t/Lib.swiftmodule \
24+
// RUN: -swift-version 5 -enable-library-evolution -O
25+
// RUN: %target-build-swift %t/main.swift -o %t/main -I%t -lLib -L%t -O
26+
// RUN: %target-codesign %t/main
27+
// RUN: %target-run %t/main | %FileCheck %s
28+
29+
/// Ensure the client doesn't have the optimization we're testing here.
30+
// RUN: %target-swift-frontend -typecheck -emit-silgen %t/main.swift -I%t -O > %t/Lib.sil
31+
// RUN: %FileCheck --check-prefix CHECK-NOT-OPTIMIZED --input-file %t/Lib.sil %s
32+
// CHECK-NOT-OPTIMIZED-NOT: _getErrorEmbeddedNSError
33+
34+
//--- optimized.swift
35+
public import Foundation
36+
37+
@inlinable public func foo<E: Error>(e: E) -> Error {
38+
return e
39+
}
40+
41+
//--- non-optimized.swift
42+
internal import Foundation
43+
44+
@inlinable public func foo<E: Error>(e: E) -> Error {
45+
return e
46+
}
47+
48+
//--- main.swift
49+
import Lib
50+
import Foundation
51+
52+
let err: NSError = NSError(domain: "Not found", code: 404)
53+
let errOut: Error = foo(e: err)
54+
print(errOut.localizedDescription)
55+
// CHECK: Not found error 404.

‎test/Serialization/generated-ref-to-nserror.swift

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,159 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: split-file %s %t --leading-lines
3+
// REQUIRES: objc_interop
4+
5+
/// Build a minimal version of Foundation defining only NSError
6+
// RUN: %target-swift-frontend -emit-module %t/Foundation.swift \
7+
// RUN: -o %t/Foundation.swiftmodule
8+
// RUN: %target-swift-frontend -emit-module %t/FoundationExporter.swift -I%t \
9+
// RUN: -o %t/FoundationExporter.swiftmodule
10+
11+
/// Enabled existential to NSError optimization
12+
// CHECK-OPTIMIZED: $NSError
13+
14+
/// Optimize Foundation itself
15+
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t \
16+
// RUN: -swift-version 5 -enable-library-evolution \
17+
// RUN: %t/Foundation.swift > %t/main.sil
18+
// RUN: %FileCheck --check-prefix CHECK-OPTIMIZED --input-file %t/main.sil %s
19+
20+
/// Public import or non-resilient modules
21+
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t \
22+
// RUN: -swift-version 5 -enable-library-evolution \
23+
// RUN: %t/inlinable-public.swift > %t/main.sil
24+
// RUN: %FileCheck --check-prefix CHECK-OPTIMIZED --input-file %t/main.sil %s
25+
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t \
26+
// RUN: %t/inlinable-public.swift > %t/main.sil
27+
// RUN: %FileCheck --check-prefix CHECK-OPTIMIZED --input-file %t/main.sil %s
28+
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t \
29+
// RUN: %t/inlinable-internal.swift > %t/main.sil
30+
// RUN: %FileCheck --check-prefix CHECK-OPTIMIZED --input-file %t/main.sil %s
31+
32+
/// Foundation is imported from a different file
33+
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t -module-name main \
34+
// RUN: %t/inlinable-not-imported-fileA.swift \
35+
// RUN: %t/inlinable-not-imported-fileB.swift > %t/main.sil
36+
// RUN: %FileCheck --check-prefix CHECK-OPTIMIZED --input-file %t/main.sil %s
37+
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t -module-name main \
38+
// RUN: -swift-version 5 -enable-library-evolution \
39+
// RUN: %t/inlinable-not-imported-fileA.swift \
40+
// RUN: %t/inlinable-not-imported-fileB.swift > %t/main.sil
41+
// RUN: %FileCheck --check-prefix CHECK-OPTIMIZED --input-file %t/main.sil %s
42+
43+
/// Foundation is imported via a transitive dependency
44+
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t -module-name main \
45+
// RUN: %t/inlinable-imported-transitive.swift > %t/main.sil
46+
// RUN: %FileCheck --check-prefix CHECK-OPTIMIZED --input-file %t/main.sil %s
47+
48+
/// Any non-inlinable uses
49+
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t \
50+
// RUN: %t/non-inlinable-public.swift > %t/main.sil
51+
// RUN: %FileCheck --check-prefix CHECK-OPTIMIZED --input-file %t/main.sil %s
52+
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t \
53+
// RUN: %t/non-inlinable-ioi.swift > %t/main.sil
54+
// RUN: %FileCheck --check-prefix CHECK-OPTIMIZED --input-file %t/main.sil %s
55+
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t \
56+
// RUN: %t/non-inlinable-internal.swift > %t/main.sil
57+
// RUN: %FileCheck --check-prefix CHECK-OPTIMIZED --input-file %t/main.sil %s
58+
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t -module-name main \
59+
// RUN: %t/non-inlinable-not-imported-fileA.swift \
60+
// RUN: %t/non-inlinable-not-imported-fileB.swift > %t/main.sil
61+
// RUN: %FileCheck --check-prefix CHECK-OPTIMIZED --input-file %t/main.sil %s
62+
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t -module-name main \
63+
// RUN: -swift-version 5 -enable-library-evolution \
64+
// RUN: %t/non-inlinable-not-imported-fileA.swift \
65+
// RUN: %t/non-inlinable-not-imported-fileB.swift > %t/main.sil
66+
// RUN: %FileCheck --check-prefix CHECK-OPTIMIZED --input-file %t/main.sil %s
67+
68+
/// Disabled existential to NSError optimization
69+
// CHECK-NOT-OPTIMIZED-NOT: $NSError
70+
71+
/// Implementation-only import
72+
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t \
73+
// RUN: %t/inlinable-ioi.swift > %t/main.sil
74+
// RUN: %FileCheck --check-prefix CHECK-NOT-OPTIMIZED --input-file %t/main.sil %s
75+
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t \
76+
// RUN: -swift-version 5 -enable-library-evolution \
77+
// RUN: %t/inlinable-ioi.swift > %t/main.sil
78+
// RUN: %FileCheck --check-prefix CHECK-NOT-OPTIMIZED --input-file %t/main.sil %s
79+
80+
/// Internal import from resilient module
81+
// RUN: %target-swift-frontend -emit-module -emit-sil -I%t \
82+
// RUN: -swift-version 5 -enable-library-evolution \
83+
// RUN: %t/inlinable-internal.swift > %t/main.sil
84+
// RUN: %FileCheck --check-prefix CHECK-NOT-OPTIMIZED --input-file %t/main.sil %s
85+
86+
//--- Foundation.swift
87+
88+
class NSError {}
89+
90+
@inlinable public func foo<E: Error>(e: E) -> Error {
91+
return e
92+
}
93+
94+
//--- FoundationExporter.swift
95+
96+
@_exported import Foundation
97+
98+
//--- inlinable-public.swift
99+
public import Foundation
100+
101+
@inlinable public func foo<E: Error>(e: E) -> Error {
102+
return e
103+
}
104+
105+
//--- inlinable-ioi.swift
106+
@_implementationOnly import Foundation
107+
108+
@inlinable public func foo<E: Error>(e: E) -> Error {
109+
return e
110+
}
111+
112+
//--- inlinable-internal.swift
113+
internal import Foundation
114+
115+
@inlinable public func foo<E: Error>(e: E) -> Error {
116+
return e
117+
}
118+
119+
//--- inlinable-not-imported-fileA.swift
120+
@inlinable public func foo<E: Error>(e: E) -> Error {
121+
return e
122+
}
123+
//--- inlinable-not-imported-fileB.swift
124+
import Foundation
125+
126+
//--- inlinable-imported-transitive.swift
127+
public import FoundationExporter
128+
129+
@inlinable public func foo<E: Error>(e: E) -> Error {
130+
return e
131+
}
132+
133+
//--- non-inlinable-public.swift
134+
public import Foundation
135+
136+
public func foo<E: Error>(e: E) -> Error {
137+
return e
138+
}
139+
140+
//--- non-inlinable-ioi.swift
141+
@_implementationOnly import Foundation
142+
143+
public func foo<E: Error>(e: E) -> Error {
144+
return e
145+
}
146+
147+
//--- non-inlinable-internal.swift
148+
internal import Foundation
149+
150+
public func foo<E: Error>(e: E) -> Error {
151+
return e
152+
}
153+
154+
//--- non-inlinable-not-imported-fileA.swift
155+
public func foo<E: Error>(e: E) -> Error {
156+
return e
157+
}
158+
//--- non-inlinable-not-imported-fileB.swift
159+
import Foundation

0 commit comments

Comments
 (0)