[SE-0252][TypeChecker] Keypath Dynamic Member Lookup #23436

xedin · 2019-03-20T01:53:48Z

Implement keypath based dynamic member lookup which augments
functionality of existing @dynamicMemberLookup attribute.

Three new subscript overloads are accepted:

subscript(dynamicMember member: KeyPath<T, ...>) -> ...
subscript(dynamicmember member: WritableKeyPath<T, ...>) -> ...
subscript(dynamicMember member: ReferenceWritableKeyPath<T, ...>) -> ...

Example:

struct Point {
  let x: Int
  var y: Int
}

@dynamicMemberLookup 
struct Lens<T> {
  var obj: T

  init(_ obj: T) {
    self.obj = obj
  }

  subscript<U>(dynamicMember member: KeyPath<T, U>) -> Lens<U> {
    get { return Lens<U>(obj[keyPath: member]) }
  }

  subscript<U>(dynamicMember member: WritableKeyPath<T, U>) -> Lens<U> {
    get { return Lens<U>(obj[keyPath: member]) }
    set { obj[keyPath: member] = newValue.obj }
  }
}

var lens = Lens(Point(x: 0, y: 0))

_ = lens.x // converted into `lens[dynamicMember: KeyPath<Point, Int>`
_ = lens.y = Lens(10) // converted into `lens[dynamicMember: WritableKeyPath<Point, Int>]`

Resolves: rdar://problem/49069813
Resolves: rdar://problem/49533404

xedin · 2019-03-20T02:20:52Z

I think I can factor out some logic from keypath constraint so I can avoid creating implicit argument expressions, but it's vital to figuring out which overload to use KeyPath or WritableKeyPath when both are present.

tkremenek · 2019-03-26T20:50:50Z

@swift-ci clean test

xedin · 2019-03-26T23:31:51Z

@swift-ci please build toolchain

xedin · 2019-03-26T23:32:36Z

@swift-ci please clean test

swift-ci · 2019-03-26T23:34:41Z

Build failed
Swift Test Linux Platform
Git Sha - 827bcfa0483c12bcc08c56839a34d14b46306834

swift-ci · 2019-03-27T00:27:05Z

Linux Toolchain (Ubuntu 16.04)
Download Toolchain
Git Sha - 45d5990e65810e0eed7d63290a4bc8233203acf4

Install command
tar zxf swift-PR-23436-182-ubuntu16.04.tar.gz
More info

xedin · 2019-03-27T02:01:07Z

@swift-ci please build toolchain

swift-ci · 2019-03-27T02:29:49Z

Build failed
Swift Test OS X Platform
Git Sha - 827bcfa0483c12bcc08c56839a34d14b46306834

xedin · 2019-03-27T08:39:10Z

@swift-ci please build toolchain

swift-ci · 2019-03-27T09:28:10Z

Linux Toolchain (Ubuntu 16.04)
Download Toolchain
Git Sha - 257ab6188fa7805ba91356b737c8b0480bea5627

Install command
tar zxf swift-PR-23436-187-ubuntu16.04.tar.gz
More info

swift-ci · 2019-03-27T10:14:16Z

macOS Toolchain
Download Toolchain
Git Sha - 257ab6188fa7805ba91356b737c8b0480bea5627

Install command
tar -zxf swift-PR-23436-243-osx.tar.gz --directory ~/

xedin · 2019-03-30T01:53:26Z

@swift-ci please build toolchain

swift-ci · 2019-03-30T02:42:30Z

Linux Toolchain (Ubuntu 16.04)
Download Toolchain
Git Sha - 702f1ea2f14d31c744a7d0d1e5f1140e7fd9d2e1

Install command
tar zxf swift-PR-23436-197-ubuntu16.04.tar.gz
More info

xedin · 2019-03-31T05:44:55Z

Looks like macOS toolchain job has finished but github missed notification.

xedin · 2019-03-31T05:45:43Z

@swift-ci please build macOS toolchain

Allow `subscript(dynamicMember: {Writeable}KeyPath<...>)` declarations to be found while looking for dynamic members.

…mic lookup

Implement keypath based dynamic member lookup which augments functionality of existing @dynamicMemberLookup attribute. Two new subscript overloads are accepted: ``` subscript(dynamicMember member: KeyPath<T, ...>) -> ... subscript(dynamicmember member: WritableKeyPath<T, ...>) -> ... ``` Example: ```swift struct Point { let x: Int var y: Int } @dynamicMemberLookup struct Lens<T> { var obj: T init(_ obj: T) { self.obj = obj } subscript(dynamicMember member: KeyPath<T, U>) -> Lens { get { return Lens(obj[keyPath: member]) } } subscript(dynamicMember member: WritableKeyPath<T, U>) -> Lens { get { return Lens(obj[keyPath: member]) } set { obj[keyPath: member] = newValue.obj } } } var lens = Lens(Point(x: 0, y: 0)) _ = lens.x // converted into `lens[dynamicMember: KeyPath<Point, Int>` _ = lens.y = Lens(10) // converted into `lens[dynamicMember: WritableKeyPath<Point, Int>]` ```

…ing based one Because keypath based choice provides more type information it should be preferred to string based when both solutions are available.

xedin · 2019-04-03T19:29:02Z

@swift-ci please test source compatibility

xedin · 2019-04-03T19:29:16Z

@swift-ci please asan test

Qbyte248 · 2019-04-03T22:06:26Z

@xedin Did you consider let lens: Lens<Lens<Point>>? What happens in this case? I would suspect two possibilities:

lens can only access the properties of the type Lens<Point>
lens can access both, the Lens<Point> and Point properties

xedin · 2019-04-03T22:12:44Z

@Qbyte248

lens can access both, the Lens<Point> and Point properties

I think this is the option makes most sense, but I don't have test-case in my suite to cover that at the moment, which I'm going to add.

… be used recursively For example if there a structure which requires multiple implicit dynamic member calls to get the members: ```swift struct Point { var x, y: Int } @dynamicMemberLookup struct Lens<T> { var obj: T subscript(dynamicMember member: KeyPath<T, U>) -> U { get { return obj[keyPath: member] } } } func foo(_ lens: Lens<Lens<Point>>) { _ = lens.x _ = lens.obj.x _ = lens.obj.obj.x } _ = \Lens<Lens<Point>>.x ```

xedin · 2019-04-04T02:48:57Z

@swift-ci please test

xedin · 2019-04-04T02:49:07Z

@swift-ci please test source compatibility

xwu · 2019-04-04T03:35:25Z

@Qbyte248

lens can access both, the Lens<Point> and Point properties

I think this is the option makes most sense, but I don't have test-case in my suite to cover that at the moment, which I'm going to add.

This option doesn't make sense to me based on the implementation of Lens:

   subscript<U>(dynamicMember member: KeyPath<T, U>) -> U {
     get { return obj[keyPath: member] }
   }

The getter does not do recursive dynamic member lookup: it only uses a plain key path subscript.

Moreover, I don't see how one can even use compiler magic to synthesize this feature in the general case (i.e., for any type other than Lens)--how exactly are you going to recursively look up dynamic members for a type NotALens<T, U, V> { var objT: T; var objU: U; var objV: V } unless you had semantic information about what NotALens is modeling?

xedin · 2019-04-04T03:40:08Z

I think original comment referred to Lens from description which does return another Lens from dynamic lookup method.

xedin · 2019-04-04T03:59:25Z

Also even if you consider subscript with returns U in case of Lens<Lens<Point>> U would be Lens<Point> and then Int so call to lens.x is equivalent to something like lens[dynamicMember: \.[dynamicMember: \.x]]

xwu · 2019-04-04T11:00:09Z

Also even if you consider subscript with returns U in case of Lens<Lens<Point>> U would be Lens<Point> and then Int so call to lens.x is equivalent to something like lens[dynamicMember: \.[dynamicMember: \.x]]

Sorry, I don’t understand. It would be like that if the getter had such an implementation (and could spell such a thing), but the getter is written in plain Swift and returns an ordinary keypath subscript that is not recursively dynamic.

xedin · 2019-04-04T17:59:02Z

@xwu Let me try to clarify, dynamic subscript here has a special behavior which tries to use root type fo the KeyPath<T, U> type to lookup members so first lookup would have Lens<Point> as a root which would find another dynamic subscript as a member, so next lookup is going to use result of Point to lookup x on that. This whole thing results in a single keypath KeyPath<Lens<Point>, Int> where subscript component (for dynamic lookup) would have an argument of property component.

I have this implemented, let me try to build a toolchain so you can try this out.

xedin · 2019-04-04T17:59:14Z

@swift-ci please build toolchain

xedin · 2019-04-04T17:59:41Z

Source compatibility failures are related to #23140

xedin · 2019-04-04T21:02:57Z

@swift-ci please ASAN test

xedin · 2019-04-04T23:48:16Z

@shahmishal Looks like toolchain reason build completed but there is probably no notification about that fact to github?

xedin · 2019-04-04T23:48:23Z

@swift-ci please ASAN test

xedin · 2019-04-04T23:48:36Z

@swift-ci please test source compatibility

xedin · 2019-04-05T07:10:48Z

Ok, looks like all of the tests are green. I talked to @DougGregor offline and we decided to do a post-merge review of these changes, so I'm going to merge everything.

xwu · 2019-04-05T14:37:45Z

which would find another dynamic subscript as a member,

Yeah this is where I disagree—or maybe misunderstand. It should not find another dynamic subscript as member.

The getter spells out the lookup as using an ordinary key path subscript, not dynamic member lookup. Unless it is your intention that, on @dynamicMemberLookup types, use of the ordinary key path subscript should also be dynamic—but that’s not a dynamic member lookup but a dynamic key path subscript lookup.

Put another way, when obj supports dynamic member lookup, obj.member is a dynamic lookup, but obj[keyPath: keyPath] shouldn’t be.

xedin · 2019-04-05T15:11:07Z

I am not exactly sure why this has anything to do with get { return obj[keyPath: member] } call in your opinion, could you please elaborate? I am trying to point out that it is possible to construct a KeyPath argument to a single dynamic member invocation in a way that would itself have a dynamic lookup as a component to reference an underlaying property if root type of the keypath is a type which supports dynamic lookup, which is the case for Lens<Lens<Point>>, where root is going to be Lens<Point>. KeyPath expression argument is going to have a single subscript component which refers to keypath dynamic member subscript with index referring to another keypath expression with a single property component pointing to x.

Are you trying to say that we shouldn’t support referencing subscript(dynamicMember:) as a keypath component?

xwu · 2019-04-06T00:26:51Z

Given let lens: Lens<Lens<Point>>:

lens.x causes invocation of the subscript
lens[dynamicMember: KeyPath<Lens<Point>, Int>] with argument \.x
The subscript getter invokes the subscript
lens.obj[keyPath: KeyPath<Lens<Point>, Int>] with argument \.x,
which is not dynamic, and not the subscript
lens.obj[dynamicMember: KeyPath<Point, Int>]

In other words, I simply don't see how the getter permits lens.x to be equivalent to lens[dynamicMember: \.[dynamicMember: \.x]]; in fact, it forbids it, since the getter makes explicit that there is no recursive dynamic member lookup.

xedin · 2019-04-06T00:33:21Z

@xwu Maybe you should just try debug toolchain and run it with -debug-constraints to see how it ends up working? Because I don't know how to explain it better and I don't understand what you are trying to say. I have this example working example:

struct Point {
  let x, y: Int
}

struct Rectangle {
  var topLeft, bottomRight: Point
}

@dynamicMemberLookup 
struct Lens<T> {
  var obj: T

  init(_ obj: T) {
    self.obj = obj
  }

  subscript<U>(dynamicMember member: KeyPath<T, U>) -> U {
    get { return obj[keyPath: member] }
  }
}

func foo(_ lens: Lens<Lens<Lens<Point>>>) {
  print(lens.x)
}

var point = Point(x: 2, y: 3)
foo(Lens(Lens(Lens(point))))

var kp = \Lens<Lens<Point>>.y
var lens = Lens(Lens(point))
print(lens[keyPath: kp])

xwu · 2019-04-06T00:52:20Z

@xwu Maybe you should just try debug toolchain and run it with -debug-constraints to see how it ends up working?

I'm pretty sure I understand how it's working. I don't understand how it can be regarded as consistent with the semantics of the protocol.

You are, as you say, making lens.x equivalent to lens[dynamicMember: \.x] or lens[dynamicMember: \.[dynamicMember: \.x]] or lens[dynamicMember: \.[dynamicMember: \.[dynamicMember: \.x]]], etc., to arbitrary levels of nesting, so long as the root of the key path is itself a type that supports dynamic member lookup by key path.

But that is not what the semantics of the protocol demand. Whether lens[dynamicMember: \.x] is or isn't equivalent to lens[dynamicMember: \.[dynamicMember: \.x]] is up to the implementation of subscript(dynamicMember member: KeyPath<T, U>) -> U.

xedin · 2019-04-06T01:03:51Z

@xwu That's kind of my point, type-checker doesn't really know how subscript(dynamicMember: KeyPath<T, U>) -> U is implemented. It only gets a declaration, opens it, and tries to combine all of the member lookups because on the rules we defined, if that leads to a solution - great! Can't really know what getter does inside from subscript use.

I'm fine saying that looking up member on Lens<Point> can't return in key path dynamic member lookup if current root type has itself been formed using key path dynamic lookup. I just don't understand why.

xedin requested a review from DougGregor March 20, 2019 01:53

tanner0101 mentioned this pull request Mar 20, 2019

add re-usable Row<Model> class vapor/fluent-kit#12

Merged

xedin force-pushed the keypath-dynamic-lookup branch 3 times, most recently from 9c21387 to 827bcfa Compare March 26, 2019 00:52

xedin changed the title ~~[TypeChecker] Implement keypath dynamic member lookup~~ [SE-0252][TypeChecker] Implement keypath dynamic member lookup Mar 26, 2019

xedin changed the title ~~[SE-0252][TypeChecker] Implement keypath dynamic member lookup~~ [SE-0252][TypeChecker] Keypath Dynamic Member Lookup Mar 26, 2019

xedin marked this pull request as ready for review March 26, 2019 23:30

xedin added the swift evolution pending discussion Flag → feature: A feature that has a Swift evolution proposal currently in review label Mar 27, 2019

xedin added 6 commits April 1, 2019 12:40

[TypeChecker] Adjust dynamic member lookup to support keypaths

b3cb1a1

Allow `subscript(dynamicMember: {Writeable}KeyPath<...>)` declarations to be found while looking for dynamic members.

[TypeChecker] Add new type of overload choice to support keypath dyna…

f95d9b0

…mic lookup

[CSApply] Implement AST transformation for keypath dynamic member lookup

71e8148

[CSRanking] Dynamic member look - rank keypath choice higher than str…

7e07644

…ing based one Because keypath based choice provides more type information it should be preferred to string based when both solutions are available.

[TypeChecker] NFC: Add tests for keypath dynamic member lookup

5295bdc

swiftlang deleted a comment from swift-ci Apr 3, 2019

swiftlang deleted a comment from swift-ci Apr 4, 2019

xedin merged commit 16b6501 into swiftlang:master Apr 5, 2019

[SE-0252][TypeChecker] Keypath Dynamic Member Lookup #23436

[SE-0252][TypeChecker] Keypath Dynamic Member Lookup #23436

Conversation

xedin commented Mar 20, 2019 • edited Loading

xedin commented Mar 20, 2019

tkremenek commented Mar 26, 2019

xedin commented Mar 26, 2019

xedin commented Mar 26, 2019

swift-ci commented Mar 26, 2019

swift-ci commented Mar 27, 2019

xedin commented Mar 27, 2019

swift-ci commented Mar 27, 2019

xedin commented Mar 27, 2019

swift-ci commented Mar 27, 2019

swift-ci commented Mar 27, 2019

xedin commented Mar 30, 2019

swift-ci commented Mar 30, 2019

xedin commented Mar 31, 2019

xedin commented Mar 31, 2019

xedin commented Apr 3, 2019

xedin commented Apr 3, 2019

Qbyte248 commented Apr 3, 2019 • edited Loading

xedin commented Apr 3, 2019

xedin commented Apr 4, 2019

xedin commented Apr 4, 2019

xwu commented Apr 4, 2019 • edited Loading

xedin commented Apr 4, 2019

xedin commented Apr 4, 2019

xwu commented Apr 4, 2019

xedin commented Apr 4, 2019

xedin commented Apr 4, 2019

xedin commented Apr 4, 2019

xedin commented Apr 4, 2019

xedin commented Apr 4, 2019

xedin commented Apr 4, 2019

xedin commented Apr 4, 2019

xedin commented Apr 5, 2019

xwu commented Apr 5, 2019

xedin commented Apr 5, 2019

xwu commented Apr 6, 2019 • edited Loading

xedin commented Apr 6, 2019

xwu commented Apr 6, 2019

xedin commented Apr 6, 2019

xedin commented Mar 20, 2019 •

edited

Loading

Qbyte248 commented Apr 3, 2019 •

edited

Loading

xwu commented Apr 4, 2019 •

edited

Loading

xwu commented Apr 6, 2019 •

edited

Loading