Skip to content

Commit e2db4d2

Browse files
committed
Revert "AST: Spot fix for AbstractStorageDecl::isResilient()"
This reverts commit d5b354f. It causes miscompiles when accessing properties declared with `@_originallyDefinedIn` that are now defined in modules with library evolution enabled from the module that the property was originally defined in. Just because the property used to be declared in the current module doesn't mean it can bypass the stable ABI of the module that the property now belongs to. It looks like the logic that this PR replaced is also faulty, since `@_originallyDefinedIn` really oughtn't factor into the resilience computation at any level, but let's unwind one level of brokenness at a time. Fixes rdar://113935401.
1 parent 5621ab3 commit e2db4d2

File tree

3 files changed

+45
-17
lines changed

3 files changed

+45
-17
lines changed

lib/AST/Decl.cpp

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2810,26 +2810,13 @@ bool AbstractStorageDecl::isResilient() const {
28102810
return getModuleContext()->isResilient();
28112811
}
28122812

2813-
static bool isOriginallyDefinedIn(const Decl *D, const ModuleDecl* MD) {
2814-
if (!MD)
2815-
return false;
2816-
if (D->getAlternateModuleName().empty())
2817-
return false;
2818-
return D->getAlternateModuleName() == MD->getName().str();
2819-
}
2820-
28212813
bool AbstractStorageDecl::isResilient(ModuleDecl *M,
28222814
ResilienceExpansion expansion) const {
28232815
switch (expansion) {
28242816
case ResilienceExpansion::Minimal:
28252817
return isResilient();
28262818
case ResilienceExpansion::Maximal:
2827-
// We consider this decl belongs to the module either it's currently
2828-
// defined in this module or it's originally defined in this module, which
2829-
// is specified by @_originallyDefinedIn
2830-
return (M != getModuleContext() &&
2831-
!isOriginallyDefinedIn(this, M) &&
2832-
isResilient());
2819+
return M != getModuleContext() && isResilient();
28332820
}
28342821
llvm_unreachable("bad resilience expansion");
28352822
}
@@ -4782,6 +4769,14 @@ DestructorDecl *NominalTypeDecl::getValueTypeDestructor() {
47824769
return cast<DestructorDecl>(found[0]);
47834770
}
47844771

4772+
static bool isOriginallyDefinedIn(const Decl *D, const ModuleDecl* MD) {
4773+
if (!MD)
4774+
return false;
4775+
if (D->getAlternateModuleName().empty())
4776+
return false;
4777+
return D->getAlternateModuleName() == MD->getName().str();
4778+
}
4779+
47854780
bool NominalTypeDecl::isResilient(ModuleDecl *M,
47864781
ResilienceExpansion expansion) const {
47874782
switch (expansion) {
@@ -4791,9 +4786,8 @@ bool NominalTypeDecl::isResilient(ModuleDecl *M,
47914786
// We consider this decl belongs to the module either it's currently
47924787
// defined in this module or it's originally defined in this module, which
47934788
// is specified by @_originallyDefinedIn
4794-
return (M != getModuleContext() &&
4795-
!isOriginallyDefinedIn(this, M) &&
4796-
isResilient());
4789+
return M != getModuleContext() && !isOriginallyDefinedIn(this, M) &&
4790+
isResilient();
47974791
}
47984792
llvm_unreachable("bad resilience expansion");
47994793
}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
@available(macOS 10.15, *)
2+
@_originallyDefinedIn(module: "OldModule", macOS 12.0)
3+
public struct OldModuleType {
4+
public let value: UInt32
5+
6+
public static let property = OldModuleType(value: 0xFFFF_FFFF)
7+
8+
public init(value: UInt32) {
9+
self.value = value
10+
}
11+
}
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: %target-swift-frontend -enable-library-evolution -module-name NewModule -emit-module-path %t/NewModule.swiftmodule -emit-module %S/Inputs/library_evolution_property_originally_defined_in_current_module_NewModule.swift
3+
// RUN: %target-swift-frontend -target %target-cpu-apple-macos12 -I %t -module-name OldModule -emit-silgen %s | %FileCheck %s
4+
5+
// REQUIRES: OS=macosx
6+
7+
import NewModule
8+
9+
@inline(never)
10+
public func use2<T>(_ t: T) {
11+
print(t)
12+
}
13+
14+
// CHECK-NOT: @{{.*}}8property{{.*}}vau :
15+
16+
public func use() {
17+
// We should access `OldModuleType.property` via its getter exported from
18+
// NewModule, even though it was _originallyDefinedIn our module.
19+
// CHECK: function_ref @{{.*}}8property{{.*}}vgZ :
20+
use2(OldModuleType.property)
21+
print(OldModuleType.property)
22+
}
23+
// CHECK-NOT: @{{.*}}8property{{.*}}vau :

0 commit comments

Comments
 (0)