[WIP] Opaque result types prototype #21137

jckarter · 2018-12-08T03:10:51Z

WIP towards a prototype of an MVP subset of https://forums.swift.org/t/opaque-result-types/15645 . For an initial implementation, this won't include the resilience, second-order where constraint, or multiple conditional conformance features proposed in the thread.

Parsing
Decl representation for opaque types
Type representation for opaque types (probably as some new kind of ArchetypeType?)
Type checking to resolve the underlying type of opaque types
Non-resilient SILGen (lowering the opaque type directly to its underlying type)
Serialization support

Limitations include:

The opaque type is spelled __opaque Protocol or __opaque ProtocolA & ProtocolB [& ProtocolC...] as placeholder spelling.
Opaque types can only appear as the return type of func declarations, not properties or closures.
where clause constraints on the opaque type are not yet supported, nor are conditional conformances on the opaque type. However, the protocols in the __opaque type can be arbitrary protocols, with any number of associated type or Self type requirements. __opaque types can in turn be inferred as associated types of protocol conformances.
The runtime and resilience aspects are not yet implemented. Changing the underlying type of a public opaque type will break ABI.

Some known issues:

Because of the missing runtime support, I substitute out opaque types during SILGen in a rather hacky unprincipled way. It's likely that the optimizer, particularly generic specialization, may still cause crashes.
The type checker currently crashes if there's a type error in the underlying type, such as if it does not conform to the required protocols.
Diagnostics involving opaque types will print the type with a compiler-internal representation involving the original decl name and generic arguments, instead of anything human-understandable.

lib/AST/ASTContext.cpp

lib/AST/GenericSignatureBuilder.cpp

include/swift/AST/Types.h

lib/AST/GenericSignatureBuilder.cpp

lib/AST/Type.cpp

xedin

Solver changes look reasonable to me, @jckarter!

xedin · 2018-12-19T23:06:04Z

lib/Sema/CSSolver.cpp

@@ -1115,6 +1115,11 @@ ConstraintSystem::solveImpl(Expr *&expr,
  // constraint.
  if (convertType) {
    auto constraintKind = ConstraintKind::Conversion;
+
+    if (getContextualTypePurpose() == CTP_ReturnStmt
+        && Options.contains(ConstraintSystemFlags::UnderlyingTypeForOpaqueReturnType))


I wonder - do we really need a new flag or we could detect this situation based on convertType itself, because it always would be OpaqueTypeArchetypeType right?

I think we normally want to treat the opaque type and underlying type as distinct outside of return statements.

Sorry, what I meant was:

if (getContextualTypePurpose() == CTP_ReturnStmt && covertType->isa<OpaqueTypeArchetypeType>()) constraintKind = ConstraintKind::OpaqueUnderlyingType;

The docs for ContextualTypePurpose suggested that it was only intended as a discriminator for diagnostics, so I wasn't sure whether it was OK to key other constraint system behavior solely on it. I can do what you suggested if you think it's better.

I think it was intended like that originally but it would be totally okay to use it for this case as well to avoid adding new flags.

xedin · 2018-12-19T23:09:57Z

lib/Sema/CSSimplify.cpp

+    }
+  }
+
+  addConstraint(ConstraintKind::Equal, type1, underlyingTyVar, locator);


Just for my better understanding, could you please elaborate if this constraint has to be equality, or it could be conversion instead? It seems like in the non-opaque case solver creates a conversion constraint from contextual result to what we infer for return, but in this case I think equality makes more sense because base is going to be a type variable with a set of requirements, is that correct? If so, I wonder if we should just bind these types instead of to pull requirements to the left type?

Yeah, the result of opening the opaque archetype should be a type variable with constraints the value of the return statement would need to satisfy. Are there cases where a Conversion constraint between type variables might still allow a more general set of types to be bound? Bind seems too specific since we still want to allow lvalues in the return statement to be loaded too.

Are there cases where a Conversion constraint between type variables might still allow a more general set of types to be bound?

I can only think of implicit conversions being a difference between equals and conversion in that regard.

Bind seems too specific since we still want to allow lvalues in the return statement to be loaded too.

Actually if you look at the matchTypes - both equal and bind share the same path and most likely going to bind types or merge equivalence classes of the type variables if either is a type variable.

jckarter · 2019-01-16T23:56:10Z

@swift-ci Please Build Toolchain macOS Platform

jckarter · 2019-01-17T03:03:17Z

@swift-ci Please build toolchain macOS platform

jckarter · 2019-01-17T03:03:35Z

@swift-ci Please build toolchain macOS platform

swift-ci · 2019-01-17T04:10:14Z

macOS Toolchain
Download Toolchain
Git Sha - e1c49799b647b49478026e032a19139ae278fa65

Install command
tar -zxf swift-PR-21137-170-osx.tar.gz --directory ~/

slavapestov

Sorry, my review is not very substantiative. But at least I read through most of the code.

include/swift/SIL/AbstractionPattern.h

lib/AST/Decl.cpp

lib/AST/GenericSignature.cpp

lib/AST/Type.cpp

lib/SILGen/SILGenConvert.cpp

test/type/opaque.swift

include/swift/AST/DiagnosticsSema.def

slavapestov · 2019-01-18T06:17:15Z

You have a binary file 'bar' that got added by mistake

jckarter · 2019-01-19T21:48:47Z

@swift-ci Please build toolchain macOS platform

jckarter · 2019-01-19T21:49:43Z

@swift-ci Please build toolchain macOS platform

swift-ci · 2019-01-19T22:58:57Z

macOS Toolchain
Download Toolchain
Git Sha - 551d6142287e3d09784e1a692dc5767913248b74

Install command
tar -zxf swift-PR-21137-174-osx.tar.gz --directory ~/

jckarter · 2019-01-22T23:33:31Z

@swift-ci Please build toolchain

To represent the abstracted interface of an opaque type, we need a generic signature that refines the outer context generic signature with an additional generic parameter representing the underlying type and its exposed constraints. Opaque types also need to be keyed by their originating decl, so that we can treat values of the same opaque type as the same. When we check a FuncDecl with an opaque type specified as its return type, create an OpaqueTypeDecl and associate it with the originating decl. (A representation for *types* derived from the opaque decl will come next.)

… bodies.

It's currently meaningless, and it'll require thought to pass the correct value when it becomes meaningful.

…lt types. This prevents opaque result types from propagating nontrivially into other declarations' types, which may be confusing and create implementation complexities.

jckarter · 2019-02-27T23:24:03Z

@swift-ci Please build toolchain macOS platform

swift-ci · 2019-02-28T03:01:07Z

macOS Toolchain
Download Toolchain
Git Sha - b63ccda

Install command
tar -zxf swift-PR-21137-216-osx.tar.gz --directory ~/

jckarter · 2019-03-21T00:58:12Z

The real implementation
#22072 can do everything this can now.

jckarter force-pushed the opaque-type-ast branch 2 times, most recently from 80fed14 to d756010 Compare December 13, 2018 19:13

DougGregor requested changes Dec 17, 2018

View reviewed changes

jckarter force-pushed the opaque-type-ast branch 2 times, most recently from 32c9c86 to 58329d2 Compare December 17, 2018 22:46

xedin reviewed Dec 19, 2018

View reviewed changes

jckarter force-pushed the opaque-type-ast branch from 5324671 to aca8292 Compare December 20, 2018 19:51

jckarter force-pushed the opaque-type-ast branch 3 times, most recently from 38e0034 to 65d32b3 Compare January 10, 2019 23:31

jckarter force-pushed the opaque-type-ast branch 2 times, most recently from 174ae36 to 2d90612 Compare January 16, 2019 23:48

jckarter force-pushed the opaque-type-ast branch from 2d90612 to e1c4979 Compare January 17, 2019 02:35

jckarter mentioned this pull request Jan 17, 2019

[5.0 WIP] Opaque types prototype #21957

Closed

slavapestov reviewed Jan 18, 2019

View reviewed changes

jckarter force-pushed the opaque-type-ast branch from ae4e289 to 260398d Compare January 18, 2019 22:06

jckarter mentioned this pull request Jan 22, 2019

[01-02-2019] Opaque result types #22042

Merged

jckarter mentioned this pull request Jan 23, 2019

Opaque types with resilience #22072

Merged

jckarter added 3 commits February 27, 2019 14:16

Parse opaque types.

7fe4f39

Add an OpaqueTypeArchetypeType subclass.

15e472a

jckarter added 3 commits February 27, 2019 14:16

Sema: Infer the underlying type for opaque return types from function…

b6a7194

… bodies.

SILGen: Substitute away opaque types.

8f5a8bc

Support nested types on opaque archetypes (and maybe opened ones).

404bd31

jckarter force-pushed the opaque-type-ast branch from b4b725b to 8e2b83f Compare February 27, 2019 22:21

jckarter added 8 commits February 27, 2019 15:23

Serialization: Serialize opaque types and their decls.

da3bdcd

IRGen: Substitute out opaque types when emitting associated types.

ad5012b

Enable opaque result types.

bd93fe6

Respond to Slava's feedback

0c0e328

Mangle opaque types without underlying types as ErrorType for now.

ef1e92e

Remove ResilienceExpansion from substOpaqueTypes for now.

8481625

It's currently meaningless, and it'll require thought to pass the correct value when it becomes meaningful.

Sema: Don't allow properties with inferred types to infer opaque resu…

8869c40

…lt types. This prevents opaque result types from propagating nontrivially into other declarations' types, which may be confusing and create implementation complexities.

Add test for deserializing opaque types across modules

b63ccda

jckarter force-pushed the opaque-type-ast branch from 8e2b83f to b63ccda Compare February 27, 2019 23:23

jckarter closed this Mar 21, 2019

[WIP] Opaque result types prototype #21137

[WIP] Opaque result types prototype #21137

Uh oh!

Conversation

jckarter commented Dec 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xedin left a comment

Choose a reason for hiding this comment

Uh oh!

xedin Dec 19, 2018

Choose a reason for hiding this comment

Uh oh!

jckarter Dec 20, 2018

Choose a reason for hiding this comment

Uh oh!

xedin Dec 20, 2018

Choose a reason for hiding this comment

Uh oh!

jckarter Dec 20, 2018

Choose a reason for hiding this comment

Uh oh!

xedin Dec 20, 2018

Choose a reason for hiding this comment

Uh oh!

xedin Dec 19, 2018

Choose a reason for hiding this comment

Uh oh!

jckarter Dec 20, 2018

Choose a reason for hiding this comment

Uh oh!

xedin Dec 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jckarter commented Jan 16, 2019

Uh oh!

jckarter commented Jan 17, 2019

Uh oh!

jckarter commented Jan 17, 2019

Uh oh!

swift-ci commented Jan 17, 2019

Uh oh!

slavapestov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

slavapestov commented Jan 18, 2019

Uh oh!

jckarter commented Jan 19, 2019

Uh oh!

jckarter commented Jan 19, 2019

Uh oh!

swift-ci commented Jan 19, 2019

Uh oh!

jckarter commented Jan 22, 2019

Uh oh!

jckarter commented Feb 27, 2019

Uh oh!

swift-ci commented Feb 28, 2019

Uh oh!

jckarter commented Mar 21, 2019

jckarter commented Dec 8, 2018 •

edited

Loading

xedin Dec 20, 2018 •

edited

Loading