Skip to content

Conversation

@nathawes
Copy link
Contributor

@nathawes nathawes commented Aug 5, 2020

This is a re-do of #33245 to account for source breakage.

Unlike \keypath expressions, the components of #keyPath() expressions weren't being resolved, so the index wouldn't pick up references for their qualifying types/properties. Semantic highlighting, cursor info, etc wouldn't work on them either. My fix for this part is largely based on @rockbruno's closed PR here: #20020 (thanks @rockbruno!)

The change to resolve ObjC #keyPath expression components caused some source breakage as they are now being checked for availability issues. To avoid that this change also updates availability checking to demote error diagnostics to warnings within #keyPath expressions. There were cases in the source compat suite where unavailable properties were used in #keyPath expressions, but worked correctly at runtime because the properties' ObjC runtime name was still correct (in this case the same as its renamed-to property in Swift).

Also fixes a code completion bug where it was reporting members from the Swift rather than ObjC side of bridged types (#keyPath(MyClass.someString.count) is invalid, but keyPath(MyClass.someString.length) is valid).

Resolves rdar://problem/61573935

Unlike \keypath expressions, only the property components of #keypath
expressions were being resolved, so index wouldn't pick up references for their
qualifying types.

Also fixes a code completion bug where it was reporting members from the Swift
rather than ObjC side of bridged types.

Resolves rdar://problem/61573935
@nathawes nathawes requested review from rintaro and xedin August 5, 2020 22:37
@nathawes
Copy link
Contributor Author

nathawes commented Aug 5, 2020

@swift-ci please test

@nathawes
Copy link
Contributor Author

nathawes commented Aug 5, 2020

@swift-ci please test source compatibility

Copy link
Contributor

@xedin xedin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add both of these new diagnostics to localization/diagnostics/en.yaml

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh neat! Will do.

@xedin xedin requested a review from CodaFi August 5, 2020 22:44
@swift-ci
Copy link
Contributor

swift-ci commented Aug 5, 2020

Build failed
Swift Test Linux Platform
Git Sha - 94e02b16801e33ac36c48b2cb2c8b6813f21b16d

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, this needs to check it's actually an ObjC rather than Swift keypath first. Fixing...

@swift-ci
Copy link
Contributor

swift-ci commented Aug 5, 2020

Build failed
Swift Test OS X Platform
Git Sha - 94e02b16801e33ac36c48b2cb2c8b6813f21b16d

… keypaths to prevent source breakage.

The change to resolve ObjC #keyPath expression components caused some source
breakage as they are now being checked for availability issues. This change
updates availability checking to demote error diagnostics to warnings
within #keyPath expressions. There were cases in the source compat suite where
unavailble properites were used in #keyPath expressions, but they caused no
issues at runtime because the properties' ObjC runtime name was still correct
(e.g. the same as its renamed-to property in Swift).
@nathawes
Copy link
Contributor Author

nathawes commented Aug 6, 2020

@swift-ci please test

@nathawes
Copy link
Contributor Author

nathawes commented Aug 6, 2020

@swift-ci please test source compatibility

@swift-ci
Copy link
Contributor

swift-ci commented Aug 6, 2020

Build failed
Swift Test Linux Platform
Git Sha - 94e02b16801e33ac36c48b2cb2c8b6813f21b16d

@swift-ci
Copy link
Contributor

swift-ci commented Aug 6, 2020

Build failed
Swift Test OS X Platform
Git Sha - 94e02b16801e33ac36c48b2cb2c8b6813f21b16d

@rintaro
Copy link
Member

rintaro commented Aug 6, 2020

Windows: Build Status

@nathawes nathawes merged commit 69abfc1 into swiftlang:master Aug 6, 2020
@nathawes nathawes deleted the index-key-paths branch August 6, 2020 22:39
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.

5 participants