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

Sema: Diagnose completely unapplied references to mutating methods. #17642

Merged

Conversation

jckarter
Copy link
Member

The currying behavior of method references completely breaks in the face of inout semantics, even moreso with exclusivity enforcement, but we failed to diagnose these references in Swift 4 and previous versions. Raise a compatibility warning when these references are found in Swift 4 code, or error in Swift 5 and later. Simplify the partial application logic here slightly too now that standalone functions do not allow currying. Addresses rdar://problem/41361334 | SR-8074.

@jckarter
Copy link
Member Author

@swift-ci Please test

@slavapestov
Copy link
Member

@jckarter Since we always miscompile to undefined behavior, a warning sounds dangerous. Why not error out unconditionally?

Copy link
Member

@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.

LGTM, but perhaps you and the rest of the core team should decide to always make this an error instead.

@swift-ci
Copy link
Collaborator

Build failed
Swift Test Linux Platform
Git Sha - cc9052890eb03565dac113bddf1a5d19685d8e94

@swift-ci
Copy link
Collaborator

Build failed
Swift Test OS X Platform
Git Sha - cc9052890eb03565dac113bddf1a5d19685d8e94

@jckarter
Copy link
Member Author

It feels a little late to spring new hard source breaks on users in 4.2 to me. I could ask though.

The currying behavior of method references completely breaks in the face of `inout` semantics, even moreso with exclusivity enforcement, but we failed to diagnose these references in Swift 4 and previous versions. Raise a compatibility warning when these references are found in Swift 4 code, or error in Swift 5 and later. Simplify the partial application logic here slightly too now that standalone functions do not allow currying. Addresses rdar://problem/41361334 | SR-8074.
@jckarter jckarter force-pushed the no-unapplied-mutating-method-references branch from cc90528 to 28fc7b0 Compare June 30, 2018 04:05
@jckarter
Copy link
Member Author

@swift-ci Please test

@swift-ci
Copy link
Collaborator

Build failed
Swift Test OS X Platform
Git Sha - cc9052890eb03565dac113bddf1a5d19685d8e94

@swift-ci
Copy link
Collaborator

Build failed
Swift Test Linux Platform
Git Sha - cc9052890eb03565dac113bddf1a5d19685d8e94

@slavapestov
Copy link
Member

@jckarter Fair enough.

@jckarter
Copy link
Member Author

The core team agreed that a compatibility warning is the right approach here.

@jckarter jckarter merged commit 1f81d54 into apple:master Jun 30, 2018
@beccadax
Copy link
Contributor

@jckarter This patch seems to break LLDB's build because it uses getStorageKind().

@jckarter
Copy link
Member Author

jckarter commented Jul 1, 2018

@brentdax This patch didn't do anything to change getStorageKind(), could you be more specific?

@beccadax
Copy link
Contributor

beccadax commented Jul 1, 2018

@jckarter You're right, I'm sorry. I must have mixed up tabs while I was tracing it down.

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.

None yet

4 participants