Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SILGen: Correctly emit vtables when an override is more visible than the base [5.1] #25190

Conversation

slavapestov
Copy link
Contributor

If an override B.f() is more visible than a base method A.f(), it is
possible that an override C.f() of B.f() cannot see the original method
A.f().

In this case, we would encounter linker errors if we referenced the
method descriptor or method dispatch thunk for A.f().

Make this work by treating B.f() as the least derived method in this
case, and ensuring that the vtable thunk for B.f() dispatches through
the vtable again.

Fixes rdar://problem/48330571, https://bugs.swift.org/browse/SR-10648.

When checking if a vtable override is ABI compatible with the
base class method, make sure to check yields too.

Also, add support for coroutines to vtable thunks, using code
that I've copy and pasted and tweaked from witness thunks.

(It would be nice to combine witness thunks and vtable thunks
into a single code path for 'method thunks', but that requires
some additional refactoring, so live with the copy and paste
for now).
We used to try to emit a 'default initializer' in a class with a superclass,
as long as the superclass was 'default initializable', meaning it had at
least one designated initializer where all parameters had a default
expression.

However this code path was never taken, because the designated initializer
inheritance mechanism supercedes it. Probably it became dead once we
implemented inheritance of initializers with default arguments.
There was a behavioral difference between binary modules and textual interfaces.
Since a binary module includes information about all members, including private
and internal members, we would emit overrides for all designated initializers
when subclassing a class, including those we cannot access.

This was problematic because then we reference the superclass initializer's
method descriptor, which had to be forced to have public linkage so that the
subclass could reference it.

However a textual interface only includes public members so any such
initializers were simply dropped. The IRGen hack is thus no longer necessary
when textual interfaces are used.

In reality neither behavior is correct because the presence of inaccessible
initializers should inhibit the inheritance of any designated initializer;
however I'm going to try to fix that separately since it is a source breaking
change and thus needs to be staged in as a warning. The latter fix is
tracked by <rdar://problem/51249311>.
…rs public linkage

This was done even for non-public inits because subclasses would always
override the base class's designated initializers, even if they were
inaccessible.

This is an ABI break, however in practice the only affected class
initializer was ManagedBuffer.init(_doNotCallMe:()), and we can just
make it @usableFromInline.
…the base

If an override B.f() is more visible than a base method A.f(), it is
possible that an override C.f() of B.f() cannot see the original method
A.f().

In this case, we would encounter linker errors if we referenced the
method descriptor or method dispatch thunk for A.f().

Make this work by treating B.f() as the least derived method in this
case, and ensuring that the vtable thunk for B.f() dispatches through
the vtable again.

Fixes <rdar://problem/48330571>, <https://bugs.swift.org/browse/SR-10648>.
@slavapestov
Copy link
Contributor Author

@swift-ci Please test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant