Embedded: allow generic class constructors #85857
Conversation
and make `AbstractFunctionDecl.isOverridden` final
We first have to remove _all_ old entries from the cache, then add the new entries. Otherwise we'll wrongly remove an existing entry from the cache again if a previous entry was deleted from the vtable.
For example:
```
public class C: MyClass {
public init(p: some P) {
// ...
}
}
```
Constructors are not called via the vtable (except "required" constructors).
Therefore we can allow generic constructors.
swiftlang#78150
rdar://138576752
eeckstein
requested review from
hborla,
jckarter,
slavapestov and
xedin
as code owners
|
@swift-ci smoke test |
eeckstein
requested review from
DougGregor,
kubamracek and
rauhul
and removed request for
hborla,
jckarter,
slavapestov and
xedin
| { | ||
| // For some reason, SILGen is putting constructors in the vtable, though they are never | ||
| // called through the vtable. | ||
| // Dropping those vtable entries allows using constructors with generic arguments. |
There was a problem hiding this comment.
Instead of working around it here, is it hard to fix SILGen to not put them in the vtable? IRGen must have a similar workaround too then.
There was a problem hiding this comment.
Instead of working around it here, is it hard to fix SILGen to not put them in the vtable?
I don't know. We (= someone who knows SILGen) can do this as a follow-up.
IRGen must have a similar workaround too then.
No, in regular SIL it's totally fine to have a generic function in a vtable. And in embedded the compiler aborted with an error in this case.
There was a problem hiding this comment.
This solution seems brittle. Why not update NeedsNewVTableEntryRequest::evaluate to suppress generic initializers from the vtable in the first place? There's no way to specialize these appropriately.
There was a problem hiding this comment.
This solution seems brittle.
No. It does not change the vtable layout. It just marks the constructor as dead. Also, we are doing this is-generic check on the call site, too (i.e. for class_method instructions) which is the double check that we are not calling such a "dead" constructor.
Why not update NeedsNewVTableEntryRequest::evaluate to suppress generic initializers from the vtable in the first place?
I tried this but did run into assertion failures in SILGen later. It's probably a more involved thing to change the vtable layout.
4 participants
For example:
Constructors are not called via the vtable (except "required" constructors).
Therefore we can allow generic constructors.
fixes: #78150
rdar://138576752