Skip to content

Conversation

@slavapestov
Copy link
Contributor

@slavapestov slavapestov commented Apr 15, 2017

The next step after #8770. Make sure we use the right reference counting style for subclass existentials and class-constrained archetypes. Also, finally remove the getExistentialTypeProtocols() utility method.

@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @jckarter and @rjmccall, why do we lower unowned(unsafe) references as "NativeObject.Type"? Does this have the right spare bits and extra inhabitants given that unowned(unsafe) can contain Objective-C tagged pointers?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. It would need to be more conservative.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jckarter @rjmccall This hack doesn't make sense -- we should just use the spare bits from the superclass constraint, if any, otherwise the spare bits of an unknown pointer, right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that would work and be a bit more optimal. If we don't already, we should also check the @_unsafe_no_tagged_pointer attribute and whether ObjC interop is enabled at all.

@slavapestov slavapestov force-pushed the subclass-existentials-irgen branch from 7831185 to f30ee81 Compare April 17, 2017 21:22
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assertion isn't valid; this code runs even when type-checking fails.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have a test case in mind I can try? I'll address this in an upcoming patch along with the other stuff above about spare bits.

Copy link
Contributor

@jrose-apple jrose-apple Apr 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think even just the obvious thing would crash:

// compile with -emit-reference-dependencies and check the resulting swiftdeps file
class MyClass {}
extension Int: (MyClass & Equatable) {}

@slavapestov slavapestov force-pushed the subclass-existentials-irgen branch from f30ee81 to 03373a1 Compare April 17, 2017 23:23
@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

@slavapestov slavapestov merged commit cc611de into swiftlang:master Apr 18, 2017
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.

3 participants