Property behaviors prototype #1297

Closed
wants to merge 5 commits into
from

Projects

None yet

2 participants

@jckarter
Member

Parse 'var [behavior] x: T', and when we see it, try to instantiate the property's
implementation in terms of the given behavior. To start out, behaviors are modeled
as protocols. If the protocol follows this pattern:

  protocol behavior {
    associatedtype Value
  }
  extension behavior {
    var value: Value { ... }
  }

then the property is instantiated by forming a conformance to behavior where
Self is bound to the enclosing type and Value is bound to the property's
declared type, and invoking the accessors of the value implementation:

  struct Foo {
    var [behavior] foo: Int
  }

  /* behaves like */

  extension Foo: private behavior {
    @implements(behavior.Value)
    private typealias `[behavior].Value` = Int

    var foo: Int {
      get { return value }
      set { value = newValue }
    }
  }

If the protocol requires a storage member, and provides an initStorage method
to provide an initial value to the storage:

  protocol storageBehavior {
    associatedtype Value

    var storage: Something<Value> { ... }
  }
  extension storageBehavior {
    var value: Value { ... }

    static func initStorage() -> Something<Value> { ... }
  }

then a stored property of the appropriate type is instantiated to witness the
requirement, using initStorage to initialize:

  struct Foo {
    var [storageBehavior] foo: Int
  }

  /* behaves like */

  extension Foo: private storageBehavior {
    @implements(storageBehavior.Value)
    private typealias `[storageBehavior].Value` = Int
    @implements(storageBehavior.storage)
    private var `[storageBehavior].storage`: Something<Int> = initStorage()

    var foo: Int {
      get { return value }
      set { value = newValue }
    }
  }

In either case, the value and storage properties should support any combination
of get-only/settable and mutating/nonmutating modifiers. The instantiated property
follows the settability and mutating-ness of the value implementation. The
protocol can also impose requirements on the Self and Value types.

Bells and whistles such as initializer expressions, accessors,
out-of-line initialization, etc. are not implemented. Additionally, behaviors
that instantiate storage are currently only supported on instance properties.
This also hasn't been tested past sema yet; SIL and IRGen will likely expose
additional issues.

@jckarter
Member

@swift-ci Please test

@jckarter
Member

@swift-ci Please test

@slavapestov slavapestov commented on the diff Feb 15, 2016
include/swift/AST/Decl.h
@@ -3626,7 +3647,7 @@ class AbstractStorageDecl : public ValueDecl {
}
};
void configureAddressorRecord(AddressorRecord *record,
- FuncDecl *addressor, FuncDecl *mutableAddressor);
+ FuncDecl *addressor, FuncDecl *mutableAddressor);
@slavapestov
slavapestov Feb 15, 2016 Member

Pointless edit here?

@jckarter
jckarter Feb 15, 2016 Member

Fixing an 80-column violation.

@slavapestov slavapestov and 1 other commented on an outdated diff Feb 15, 2016
lib/AST/Decl.cpp
@@ -3300,6 +3310,20 @@ ParamDecl::ParamDecl(ParamDecl *PD)
/// \brief Retrieve the type of 'self' for the given context.
+Type DeclContext::getSelfTypeInContext() const {
+ // For a protocol or extension thereof, the type is 'Self'.
+ if (getAsProtocolOrProtocolExtensionContext()) {
+ // In the parser, generic parameters won't be wired up yet, just give up on
+ // producing a type.
+ if (getGenericParamsOfContext() == nullptr)
+ return Type();
+ return getProtocolSelf()->getArchetype();
+ }
+ return getDeclaredTypeInContext();
+}
+
+/// \brief Retrieve the type of 'self' for the given context.
+/// FIXME: Can this be integrated with getSelfTypeInContext above?
static Type getSelfTypeForContext(DeclContext *dc) {
@slavapestov
slavapestov Feb 15, 2016 Member

There doesn't seem to be any difference between the two forms of getSelfTypeInContext().

@jckarter
jckarter Feb 15, 2016 Member

The static implementation returns the unbound generic type. It has only one use, and I didn't want to disturb it.

@slavapestov
slavapestov Feb 15, 2016 Member

Oh, getDeclaredTypeInContext() vs getDeclaredTypeOfContext(). Ugh...

@slavapestov slavapestov commented on an outdated diff Feb 15, 2016
lib/Sema/CodeSynthesis.cpp
+
+ // Mark the vardecl to be final, implicit, and private. In a class, this
+ // prevents it from being dynamically dispatched.
+ if (VD->getDeclContext()->getAsClassOrClassExtensionContext())
+ makeFinal(Context, Storage);
+ Storage->setImplicit();
+ Storage->setAccessibility(Accessibility::Private);
+ Storage->setSetterAccessibility(Accessibility::Private);
+
+ // Add accessors to the storage, since we'll need them to satisfy the
+ // conformance requirements.
+ addTrivialAccessorsToStorage(Storage, *this);
+
+ // Add the witnesses to the conformance.
+ ArrayRef<Substitution> MemberSubs;
+ if (DC->isTypeContext() && DC->isGenericContext()) {
@slavapestov
slavapestov Feb 15, 2016 Member

DC->isGenericTypeContext()

@slavapestov slavapestov and 1 other commented on an outdated diff Feb 15, 2016
lib/Sema/CodeSynthesis.cpp
+ // Mark the vardecl to be final, implicit, and private. In a class, this
+ // prevents it from being dynamically dispatched.
+ if (VD->getDeclContext()->getAsClassOrClassExtensionContext())
+ makeFinal(Context, Storage);
+ Storage->setImplicit();
+ Storage->setAccessibility(Accessibility::Private);
+ Storage->setSetterAccessibility(Accessibility::Private);
+
+ // Add accessors to the storage, since we'll need them to satisfy the
+ // conformance requirements.
+ addTrivialAccessorsToStorage(Storage, *this);
+
+ // Add the witnesses to the conformance.
+ ArrayRef<Substitution> MemberSubs;
+ if (DC->isTypeContext() && DC->isGenericContext()) {
+ MemberSubs = DC->getDeclaredTypeInContext()->getAs<BoundGenericType>()
@slavapestov
slavapestov Feb 15, 2016 Member

Is DC->getGenericParamsOfContext()->getForwardingSubstitutions() more correct if we ever do nested generics?

@jckarter
jckarter Feb 15, 2016 Member

Thanks, I was looking for that and couldn't find it.

@slavapestov slavapestov commented on an outdated diff Feb 15, 2016
lib/Sema/CodeSynthesis.cpp
+
+ auto InitStorageRef = new (Context) DeclRefExpr(SpecializeInitStorage,
+ DeclNameLoc(),
+ /*implicit*/ true);
+ auto InitStorageMethodTy = FunctionType::get(Context.TheEmptyTupleType,
+ SubstStorageContextTy);
+ auto InitStorageRefTy = FunctionType::get(SelfTypeRef->getType(),
+ InitStorageMethodTy);
+ InitStorageRef->setType(InitStorageRefTy);
+
+ auto SelfApply = new (Context) DotSyntaxCallExpr(InitStorageRef,
+ SourceLoc(),
+ SelfTypeRef);
+ SelfApply->setImplicit();
+ SelfApply->setType(InitStorageMethodTy);
+ auto EmptyTuple = TupleExpr::create(Context,
@slavapestov
slavapestov Feb 15, 2016 Member

TupleExpr::createEmpty()?

@slavapestov slavapestov commented on an outdated diff Feb 15, 2016
lib/Sema/CodeSynthesis.cpp
+ if (dc->isTypeContext()) {
+ behaviorSelf = dc->getSelfTypeInContext();
+ assert(behaviorSelf && "type context doesn't have self type?!");
+ if (var->isStatic())
+ behaviorSelf = MetatypeType::get(behaviorSelf);
+ } else {
+ behaviorSelf = TC.Context.TheEmptyTupleType;
+ }
+
+ conformance = TC.Context.getBehaviorConformance(behaviorSelf,
+ behaviorProto,
+ behavior->getLoc(), dc,
+ ProtocolConformanceState::Checking);
+ }
+ make_behavior_accessors:
+ FuncDecl *getter = nullptr, *setter = nullptr;
@slavapestov
slavapestov Feb 15, 2016 Member

The only free variables in the block after the label are 'var', 'valueProp', 'conformance' and 'TC'. If you move this into its own function you won't need the gotos. This would move the var->setIsBeingTypeCheckedCalls() calls closer together which is good IMHO

@slavapestov slavapestov commented on the diff Feb 15, 2016
lib/Sema/CodeSynthesis.cpp
@@ -1332,6 +1685,10 @@ void swift::maybeAddAccessorsToVariable(VarDecl *var, TypeChecker &TC) {
return;
}
+ // Local variables don't otherwise get accessors.
+ if (var->getDeclContext()->isLocalContext())
@slavapestov
slavapestov Feb 15, 2016 Member

Was there a test case for this fix?

@jckarter
jckarter Feb 15, 2016 Member

I test some local vars with behaviors, yeah. We ban lazy in local contexts so it doesn't yet matter in trunk.

@slavapestov slavapestov commented on an outdated diff Feb 15, 2016
lib/Sema/TypeCheckDecl.cpp
+
+ auto behavior = decl->getMutableBehavior() ;
+ // We should have set up the conformance during validation.
+ assert(behavior->Conformance.hasValue());
+ // If the behavior couldn't be resolved during validation, we can't really
+ // do much more.
+ auto *conformance = behavior->Conformance.getValue();
+ if (!conformance)
+ return;
+
+ assert(behavior->ValueDecl);
+
+ auto dc = decl->getDeclContext();
+ auto behaviorSelf = conformance->getType();
+ auto behaviorProto = conformance->getProtocol();
+ auto behaviorProtoTy = ProtocolType::get(behaviorProto, TC.Context);
@slavapestov
slavapestov Feb 15, 2016 Member

Is this not behaviorProto->getDeclaredTypeInContext()?

@slavapestov slavapestov commented on the diff Feb 15, 2016
lib/Sema/TypeCheckDecl.cpp
+ TC.diagnose(behavior->getLoc(),
+ diag::property_behavior_with_self_requirement_not_in_type,
+ behaviorProto->getName());
+ break;
+ }
+
+ // Blame conformance failures on the containing type.
+ SourceLoc blameLoc;
+ if (auto nomTy = dyn_cast<NominalTypeDecl>(dc)) {
+ blameLoc = nomTy->getLoc();
+ } else if (auto ext = dyn_cast<ExtensionDecl>(dc)) {
+ blameLoc = ext->getLoc();
+ } else {
+ llvm_unreachable("unknown type context type?!");
+ }
+ bool conforms = TC.conformsToProtocol(behaviorSelf, refinedProto, dc,
@slavapestov
slavapestov Feb 15, 2016 Member

Indentation

@slavapestov slavapestov commented on the diff Feb 15, 2016
lib/Sema/TypeCheckDecl.cpp
+
+ // Bail out if we didn't resolve method witnesses.
+ if (conformance->isInvalid()) {
+ decl->setInvalid();
+ decl->overwriteType(ErrorType::get(TC.Context));
+ return;
+ }
+
+ conformance->setState(ProtocolConformanceState::Complete);
+
+ // Check that the 'value' property from the protocol matches the
+ // declared property type in context.
+ auto sig = behaviorProto->getGenericSignatureOfContext();
+ auto map = sig->getSubstitutionMap(interfaceSubs);
+ auto substValueTy = behavior->ValueDecl->getInterfaceType()
+ .subst(decl->getModuleContext(), map, SubstOptions());
@slavapestov
slavapestov Feb 15, 2016 Member

I think SubstOptions are a default parameter?

@jckarter
jckarter Feb 15, 2016 Member

Doesn't seem to be. I was just following Xcode code completion, which swallows defaulted args.

@slavapestov slavapestov and 1 other commented on an outdated diff Feb 15, 2016
test/decl/var/behaviors.swift
@@ -0,0 +1,193 @@
+// RUN: %target-parse-verify-swift -module-name Main
+
+protocol behavior {
+ associatedtype Value
@slavapestov
slavapestov Feb 15, 2016 Member

Does it work to give this a default type? What about a protocol constraint

@jckarter
jckarter Feb 15, 2016 Member

Yeah, those both ought to work. My hacked-up hand-rolled conformances probably won't handle new associated type requirements yet if constraints on Value introduce them.

@slavapestov
Member

@jckarter This looks awesome. I just have a few nitpicks that you're free to ignore.

@jpsim jpsim referenced this pull request in realm/realm-cocoa Feb 15, 2016
Closed

A Plain Old <Insert your language> Objects approach #3212

@jckarter
Member

@swift-ci Please test

@jckarter
Member

@swift-ci Please test

jckarter added some commits Jan 28, 2016
@jckarter jckarter Sema: Initial parsing and synthesis for properties with behaviors.
Parse 'var [behavior] x: T', and when we see it, try to instantiate the property's
implementation in terms of the given behavior. To start out, behaviors are modeled
as protocols. If the protocol follows this pattern:

  ```
  protocol behavior {
    associatedtype Value
  }
  extension behavior {
    var value: Value { ... }
  }
  ```

then the property is instantiated by forming a conformance to `behavior` where
`Self` is bound to the enclosing type and `Value` is bound to the property's
declared type, and invoking the accessors of the `value` implementation:

  ```
  struct Foo {
    var [behavior] foo: Int
  }

  /* behaves like */

  extension Foo: private behavior {
    @implements(behavior.Value)
    private typealias `[behavior].Value` = Int

    var foo: Int {
      get { return value }
      set { value = newValue }
    }
  }
  ```

If the protocol requires a `storage` member, and provides an `initStorage` method
to provide an initial value to the storage:

  ```
  protocol storageBehavior {
    associatedtype Value

    var storage: Something<Value> { ... }
  }
  extension storageBehavior {
    var value: Value { ... }

    static func initStorage() -> Something<Value> { ... }
  }
  ```

then a stored property of the appropriate type is instantiated to witness the
requirement, using `initStorage` to initialize:

  ```
  struct Foo {
    var [storageBehavior] foo: Int
  }

  /* behaves like */

  extension Foo: private storageBehavior {
    @implements(storageBehavior.Value)
    private typealias `[storageBehavior].Value` = Int
    @implements(storageBehavior.storage)
    private var `[storageBehavior].storage`: Something<Int> = initStorage()

    var foo: Int {
      get { return value }
      set { value = newValue }
    }
  }
  ```

In either case, the `value` and `storage` properties should support any combination
of get-only/settable and mutating/nonmutating modifiers. The instantiated property
follows the settability and mutating-ness of the `value` implementation. The
protocol can also impose requirements on the `Self` and `Value` types.

Bells and whistles such as initializer expressions, accessors,
out-of-line initialization, etc. are not implemented. Additionally, behaviors
that instantiate storage are currently only supported on instance properties.
This also hasn't been tested past sema yet; SIL and IRGen will likely expose
additional issues.
32f0e43
@jckarter jckarter Sema/SILGen: Get property behavior implementations to codegen.
Fix some interface type/context type confusion in the AST synthesis from the previous patch, add a unique private mangling for behavior protocol conformances, and set up SILGen to emit the conformances when property declarations with behaviors are visited. Disable synthesis of the struct memberwise initializer if any instance properties use behaviors; codegen will need to be redesigned here.
648ae57
@jckarter jckarter Add an interpreter test for "delayed" property behavior.
We have enough implementation to do "delayed" now.
620c931
@jckarter jckarter changed the title from Sema: Initial parsing and synthesis for properties with behaviors. to Property behaviors prototype Feb 19, 2016
@jckarter
Member

@swift-ci Please test

jckarter added some commits Feb 17, 2016
@jckarter jckarter Sema: Fix crash when behavior protocol is missing `initStorage` requi…
…rement.

Would be nice to have `guard` in C++...
121d44e
@jckarter jckarter Conditionally enable behaviors by a frontend flag.
Since the feature is incomplete and yet to be accepted or implemented as proposed, hide it behind an -enable-experimental-property-behaviors frontend flag.
79fbbaf
@jckarter
Member

@swift-ci Please test

@jckarter
Member

I've merged this into master, hidden behind a frontend flag, -enable-experimental-property-behaviors.

@jckarter jckarter closed this Feb 21, 2016
@jckarter jckarter deleted the jckarter:property-behaviors branch Feb 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment