Skip to content

[4.1] For 'lazy', make "cannot override with a stored property" a warning #13308

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Dec 7, 2017
Merged

[4.1] For 'lazy', make "cannot override with a stored property" a warning #13308

merged 1 commit into from
Dec 7, 2017

Conversation

jrose-apple
Copy link
Contributor

  • Explanation: Previous versions of Swift accidentally treated lazy properties as computed properties because of how they were implemented. Now that we check this correctly, we've broken source compatibility. Downgrade the error to a warning in this case.
  • Scope: Only affects properties marked both override and lazy.
  • Issue: rdar://problem/35870371
  • Reviewed by: @slavapestov
  • Risk: Very low. An error becomes a warning under a very safe condition. (The original change could have other effects we haven't discovered, but that's not the risk being evaluated here.)
  • Testing: Added compiler regression tests.

@jrose-apple
For 'lazy', make "cannot override with a stored property" a warning (s…
1f8b232 
…wiftlang#13304)

Previous versions of Swift accidentally treated lazy properties as
computed properties because of how they were implemented. Now that we
check this correctly, we've broken source compatibility. Downgrade the
error to a warning in this case.

(Arguably we could /allow/ overriding with a stored property. The
original concerns were that you could accidentally end up with extra
storage you didn't need, and that observing accessors would behave
differently based on whether or not the property was overriding. But
there's at least no ambiguity for 'lazy', which can't have observing
accessors today.)

rdar://problem/35870371
(cherry picked from commit 7c707ce)
DougGregor
DougGregor approved these changes Dec 6, 2017
View reviewed changes
Copy link
Member

@DougGregor DougGregor left a comment

Choose a reason for hiding this comment

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

LGTM!

@ematejska
Copy link
Contributor

@swift-ci please test

@swift-ci
Copy link
Contributor

swift-ci commented Dec 6, 2017

Build failed
Swift Test Linux Platform
Git Sha - 1f8b232

@shahmishal
Copy link
Member

@swift-ci please test macOS

@jrose-apple
Copy link
Contributor Author

swiftlang/swift-corelibs-foundation#1356

@swift-ci test Linux

@swift-ci
Copy link
Contributor

swift-ci commented Dec 7, 2017

Build failed
Swift Test Linux Platform
Git Sha - 1f8b232

@jrose-apple
Copy link
Contributor Author 

error: use of unresolved identifier '_swift_getDefaultErrorCode'
        return _swift_getDefaultErrorCode(self)
               ^~~~~~~~~~~~~~~~~~~~~~~~~~
Swift._getDefaultErrorCode:3:13: note: did you mean '_getDefaultErrorCode'?
public func _getDefaultErrorCode<T>(_ error: T) -> Int where T : Error
            ^

Looks like there's more than one outstanding corelibs PR before the tests will pass again.

@shahmishal
Copy link
Member

@swift-ci test Linux

@jrose-apple jrose-apple merged commit 8b05b0b into swiftlang:swift-4.1-branch Dec 7, 2017
@jrose-apple jrose-apple deleted the 4.1-lazy-override-lax-enforcement branch December 7, 2017 16:54
@jrose-apple jrose-apple mentioned this pull request Dec 11, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DougGregor DougGregor DougGregor approved these changes

@slavapestov slavapestov Awaiting requested review from slavapestov

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jrose-apple @ematejska @swift-ci @shahmishal @DougGregor