Skip to content
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

Exclude properties of class with type Self #23485

Merged
merged 5 commits into from
Mar 27, 2019

Conversation

johnno1962
Copy link
Contributor

@johnno1962 johnno1962 commented Mar 22, 2019

Hi Apple,

as a followup to #22863, prevent people from declaring properties of type Self which seems to not work particularly well as mentioned in the Jira below and the previous PR. This isn't really a solution but prevents compiler crashes and erratic diagnostics. cc @slavapestov. It now gives the error below when a user tries to declare a property with type Self in a class:

h.swift:155:15: error: 'Self' is only available in a protocol or as the result of a method in a class; did you mean 'A1'?
  var copied: Self {
              ^~~~
              A1

Resolves #52537.

@johnno1962 johnno1962 changed the title Exclude properties with type Self [DNM] Exclude properties with type Self Mar 22, 2019
@xwu
Copy link
Collaborator

xwu commented Mar 22, 2019

Is there a reason to exclude this for concrete value types?

@johnno1962
Copy link
Contributor Author

Value types work so I've pushed another commit. @xwu, are you able to run tests for me please?

@johnno1962 johnno1962 changed the title [DNM] Exclude properties with type Self Exclude properties of class with type Self Mar 22, 2019
@johnno1962
Copy link
Contributor Author

johnno1962 commented Mar 22, 2019

I've re-synced to apple/master and tests now pass so this change should be ready to merge.

@xwu
Copy link
Collaborator

xwu commented Mar 23, 2019

@swift-ci Please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - c317fda

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - c317fda

@johnno1962
Copy link
Contributor Author

👍, thanks @xwu, LGTM.

@johnno1962
Copy link
Contributor Author

@swift-ci Please test

@johnno1962
Copy link
Contributor Author

@xwu, when you have a spare moment could you @swift-ci Please test again for me?

@xwu
Copy link
Collaborator

xwu commented Mar 24, 2019

@swift-ci Please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 40016d5

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 40016d5

@johnno1962
Copy link
Contributor Author

Thanks @xwu, over to you @slavapestov

Copy link
Contributor

@slavapestov slavapestov left a comment

Choose a reason for hiding this comment

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

If you want, you can try to lift the restriction on get-only properties returning Self in a subsequent PR. It shouldn't be difficult!

@slavapestov slavapestov merged commit 175acc6 into swiftlang:master Mar 27, 2019
@johnno1962
Copy link
Contributor Author

johnno1962 commented Mar 27, 2019

Thanks @slavapestov, another time perhaps. Would you be able to cherry pick it to 5.1 for me please? I've exhausted my git skills with the fork I have which doesn't have a swift-5.1-branch.

@slavapestov
Copy link
Contributor

Sure.

@slavapestov
Copy link
Contributor

Here you go: #23613

@johnno1962
Copy link
Contributor Author

ta!

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