-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[SourceKit] SourceKit support for opaque result types #24177
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
[SourceKit] SourceKit support for opaque result types #24177
Conversation
rdar://problem/49353647
rdar://problem/49354663
rdar://problem/49354106
rdar://problem/49819227
rdar://problem/49354663
rdar://problem/49398426
@swift-ci Please smoke test |
isSimple = false; | ||
break; | ||
case PrintOptions::OpaqueReturnTypePrintingMode::WithoutOpaqueKeyword: { | ||
isSimple = opaqueTy->getConformsTo().size() < 2; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can an opaque type have a superclass and one conformance? CC @jckarter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, an opaque type can have any combination of constraints.
LLVM_FALLTHROUGH; | ||
case PrintOptions::OpaqueReturnTypePrintingMode::WithoutOpaqueKeyword: { | ||
SmallVector<Type, 2> types; | ||
for (auto proto : T->getConformsTo()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about T->getSuperclass()?
Maybe this should be generalized to get all the constraints from T->getBoundSignature()
instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will fix.
types.push_back(proto->TypeDecl::getDeclaredInterfaceType()); | ||
|
||
// Create and visit temporary ProtocolCompositionType. | ||
auto composition = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a more direct way to turn an opaque type into an existential, @jckarter ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added ArchetypeType::getExistentialType()
in #24166.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jckarter Thanks! I'll modify ASTPrinter to use that too.
// If resolved print it. | ||
return nullptr; | ||
|
||
return assocTyD->getInherited()[0]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if getInherited() has multiple entries?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
associatedtype Aaaa: Collection, Comparable
Thanks! I didn't know this is accepted by the compiler.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fact the inheritance clause doesn’t contain all the information you need. I would suggest asking the protocol’s generic signature instead.
Eg, protoDecl->getGenericSignature()->getConformsTo(assocDecl->getDeclaredInterfaceType())
You should also check for a superclass and layout constraint, so you can print ‘some SomeClass’ or ‘some AnyObject & P’, etc.
else | ||
return nullptr; | ||
|
||
if (!ResultT->is<DependentMemberType>()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what this function is doing. Are you printing protocol requirements and extension methods that return associated types as returning an opaque type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, for example:
protocol P {
associatedtype Assoc: Collection
func foo() -> Assoc
}
protocol C: P {
func /* Complete here. */
}
We wan't to complete foo()
as func foo() -> some Collection { <#code#> }
Initial implementation for SourceKit support for "opaque result types"
some
keywordrdar://problem/49997412