Skip to content

[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

Merged
merged 8 commits into from
Apr 22, 2019

Conversation

rintaro
Copy link
Member

@rintaro rintaro commented Apr 20, 2019

Initial implementation for SourceKit support for "opaque result types"

  • libSyntax: Grammer and parser change
  • Syntax highlighting, Cursor Info, Indexing
  • Code completion for some keyword
  • Conformance completion using opaque result types

rdar://problem/49997412

@rintaro
Copy link
Member Author

rintaro commented Apr 20, 2019

@swift-ci Please smoke test

@rintaro rintaro merged commit 73d3fd6 into swiftlang:master Apr 22, 2019
@rintaro rintaro deleted the sourcekit-opaque-result-types branch April 22, 2019 16:58
isSimple = false;
break;
case PrintOptions::OpaqueReturnTypePrintingMode::WithoutOpaqueKeyword: {
isSimple = opaqueTy->getConformsTo().size() < 2;
Copy link
Contributor

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

Copy link
Contributor

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())
Copy link
Contributor

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?

Copy link
Member Author

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 =
Copy link
Contributor

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 ?

Copy link
Contributor

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.

Copy link
Member Author

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];
Copy link
Contributor

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?

Copy link
Member Author

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.

Copy link
Contributor

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>())
Copy link
Contributor

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?

Copy link
Member Author

@rintaro rintaro Apr 22, 2019

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#> }

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.

4 participants