Skip to content

Commit 27a81dc

Browse files
tkremenektkremenek
committed
Merge pull request #1954 from jckarter/rdar25412647-v2-2.2
SIL devirtualization: Conservatively avoid filtering devirt candidates on generic base classes.
2 parents 04b09d1 + 29b561f commit 27a81dc

File tree

5 files changed

+126
-5
lines changed

5 files changed

+126
-5
lines changed

‎lib/SIL/Verifier.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2006,7 +2006,11 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
20062006
"downcast operand must be a class type");
20072007
require(toCanTy.getClassOrBoundGenericClass(),
20082008
"downcast must convert to a class type");
2009-
require(SILType::getPrimitiveObjectType(fromCanTy).
2009+
// FIXME: "isSuperclassOf" incorrectly rejects subclasses of
2010+
// specializations of generics. As a spot fix for 2.2, relax the verifier
2011+
// here. This is fixed properly in master.
2012+
require(isa<BoundGenericType>(fromCanTy) ||
2013+
SILType::getPrimitiveObjectType(fromCanTy).
20102014
isSuperclassOf(SILType::getPrimitiveObjectType(toCanTy)),
20112015
"downcast must convert to a subclass");
20122016
}

‎lib/SILOptimizer/Transforms/SpeculativeDevirtualizer.cpp

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -377,8 +377,16 @@ static bool tryToSpeculateTarget(FullApplySite AI,
377377
return false;
378378
// Handle the usual case here: the class in question
379379
// should be a real subclass of a bound generic class.
380-
return !ClassType.isSuperclassOf(
381-
SILType::getPrimitiveObjectType(SubCanTy));
380+
// FIXME: `isSuperclassOf` is the wrong check for a generic class,
381+
// since a subclass may inherit a more specific instantiation of the
382+
// generic, as in:
383+
// class Base<T> {}
384+
// class Sub: Base<Int> {}
385+
// class Sub2<U: Foo>: Base<U> {}
386+
// This is more comprehensively fixed in Swift 3. As a spot fix for
387+
// Swift 2.2, avoid filtering subclasses of generic bases.
388+
return !ClassType.is<BoundGenericType>() &&
389+
!ClassType.isSuperclassOf(SILType::getPrimitiveObjectType(SubCanTy));
382390
});
383391
Subs.erase(RemovedIt, Subs.end());
384392
}

‎lib/SILOptimizer/Utils/Devirtualize.cpp

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,16 @@ static void getAllSubclasses(ClassHierarchyAnalysis *CHA,
7373
return false;
7474
// Handle the usual case here: the class in question
7575
// should be a real subclass of a bound generic class.
76-
return !ClassType.isSuperclassOf(
77-
SILType::getPrimitiveObjectType(SubCanTy));
76+
// FIXME: `isSuperclassOf` is the wrong check for a generic class,
77+
// since a subclass may inherit a more specific instantiation of the
78+
// generic, as in:
79+
// class Base<T> {}
80+
// class Sub: Base<Int> {}
81+
// class Sub2<U: Foo>: Base<U> {}
82+
// This is more comprehensively fixed in Swift 3. As a spot fix for
83+
// Swift 2.2, avoid filtering subclasses of generic bases.
84+
return !ClassType.is<BoundGenericType>() &&
85+
!ClassType.isSuperclassOf(SILType::getPrimitiveObjectType(SubCanTy));
7886
});
7987
Subs.erase(RemovedIt, Subs.end());
8088
}

‎test/Interpreter/classes.swift

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,3 +170,23 @@ print((b as Bank).transferMoney(Account(owner: "A"), to: Account(owner: "B")))
170170
print((b as Bank).transferMoney(nil, to: nil))
171171
print((b as Bank).deposit(Account(owner: "Cyberdyne Systems")))
172172
print((b as Bank).deposit(Account(owner: "A")))
173+
174+
// rdar://25412647
175+
176+
private class Parent <T> {
177+
func doSomething() {
178+
overriddenMethod()
179+
}
180+
181+
func overriddenMethod() {
182+
fatalError("You should override this method in child class")
183+
}
184+
}
185+
186+
private class Child: Parent<String> {
187+
override func overriddenMethod() {
188+
print("Heaven!")
189+
}
190+
}
191+
192+
Child().doSomething() // CHECK: Heaven!

‎test/SILOptimizer/devirt_concrete_subclass_of_generic_class.swift

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,3 +56,84 @@ print(test3())
5656
print(test4(Derived()))
5757

5858
print(test5(Derived()))
59+
60+
// Check that we handle indirect devirtualization through an intermediate
61+
// method. rdar://problem/24993618
62+
63+
private class IndirectMethodCall<T> {
64+
func bug() {
65+
overrideMe()
66+
}
67+
68+
@inline(never)
69+
func overrideMe() { }
70+
}
71+
72+
private class IndirectChildConcrete: IndirectMethodCall<Int> {
73+
@inline(never)
74+
override func overrideMe() { }
75+
}
76+
77+
private class IndirectChildTuple<U>: IndirectMethodCall<(U, U)> {
78+
@inline(never)
79+
override func overrideMe() { }
80+
}
81+
82+
private class IndirectChildTupleConcrete: IndirectChildTuple<Int> {
83+
@inline(never)
84+
override func overrideMe() { }
85+
}
86+
87+
private class IndirectChildMeta<U>: IndirectMethodCall<U.Type> {
88+
@inline(never)
89+
override func overrideMe() { }
90+
}
91+
private class IndirectChildMetaConcrete: IndirectChildMeta<Int> {
92+
@inline(never)
93+
override func overrideMe() { }
94+
}
95+
96+
private class IndirectChildBoundGeneric<U>: IndirectMethodCall<Array<U>> {
97+
@inline(never)
98+
override func overrideMe() { }
99+
}
100+
101+
private class IndirectChildBoundGenericConcrete:
102+
IndirectChildBoundGeneric<Int> {
103+
@inline(never)
104+
override func overrideMe() { }
105+
}
106+
107+
private class IndirectChildFunction<U>: IndirectMethodCall<U -> U> {
108+
@inline(never)
109+
override func overrideMe() { }
110+
}
111+
private class IndirectChildFunctionConcrete: IndirectChildFunction<Int> {
112+
@inline(never)
113+
override func overrideMe() { }
114+
}
115+
116+
// CHECK-LABEL: sil {{.*}} @{{.*}}test6
117+
@inline(never)
118+
func test6() {
119+
// CHECK: function_ref @{{.*}}IndirectChildConcrete{{.*}}overrideMe
120+
IndirectChildConcrete().bug()
121+
// CHECK: function_ref @{{.*}}IndirectChildTuple{{.*}}overrideMe
122+
IndirectChildTuple<Int>().bug()
123+
// CHECK: function_ref @{{.*}}IndirectChildTupleConcrete{{.*}}overrideMe
124+
IndirectChildTupleConcrete().bug()
125+
// CHECK: function_ref @{{.*}}IndirectChildMeta{{.*}}overrideMe
126+
IndirectChildMeta<Int>().bug()
127+
// CHECK: function_ref @{{.*}}IndirectChildMetaConcrete{{.*}}overrideMe
128+
IndirectChildMetaConcrete().bug()
129+
// CHECK: function_ref @{{.*}}IndirectChildBoundGeneric{{.*}}overrideMe
130+
IndirectChildBoundGeneric<Int>().bug()
131+
// CHECK: function_ref @{{.*}}IndirectChildBoundGenericConcrete{{.*}}overrideMe
132+
IndirectChildBoundGenericConcrete().bug()
133+
// CHECK: function_ref @{{.*}}IndirectChildFunction{{.*}}overrideMe
134+
IndirectChildFunction<Int>().bug()
135+
// CHECK: function_ref @{{.*}}IndirectChildFunctionConcrete{{.*}}overrideMe
136+
IndirectChildFunctionConcrete().bug()
137+
}
138+
139+
print(test6())

0 commit comments

Comments
 (0)