Sema: Three fixes for the new @escaping attribute (3.0) #4388
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
slavapestov
commented
Aug 18, 2016
slavapestov
commented
- Explanation: This patch fixes some issues that were reported with the change to make non-escaping closures the default and add an @escaping attribute. In particular, there was no way to have a non-escaping parameter of function type if the function type was an alias, or parenthesized. Furthermore, @escaping could not be applied to these types, and @escaping in invalid locations was not diagnosed and had no effect.
- Scope: Affects anyone writing code that operates on closures. This is a source-breaking change, and the current behavior is counter-intuititive, so this needs to be fixed.
- Risk: Medium.
- Testing: New tests added, existing tests updated for the new, more correct semantics of @escaping.
- Bugs: https://bugs.swift.org/browse/SR-2053, https://bugs.swift.org/browse/SR-2397
- If a parameter type is a sugared function type, mark the type as non-escaping by default. Previously, we were only doing this if the parameter type was written as a function type, with no additional sugar. This means in the following cases, the function parameter type is now non-escaping: func foo(f: ((Int) -> Void)) typealias Fn = (Int) -> Void func foo(f: Fn) - Also, allow @escaping to be used in the above cases: func foo(f: @escaping ((Int) -> Void)) typealias Fn = (Int) -> Void func foo(f: @escaping Fn) - Diagnose usages of @escaping in inappropriate locations, instead of just ignoring them. It is unfortunate that sometimes we end up desugaring the typealias, but currently there are other cases where this occurs too, such as qualified lookup of protocol typealiases with a concrete base type, and generic type aliases. A more general representation for sugared types (such as an AttributedType sugared type) would allow us to solve this in a more satisfactory manner in the future. However at the very least this patch factors out the common code paths and adds comments, so it shouldn't be too bad going forward. Note that this is a source-breaking change, both because @escaping might need to be added to parameters with a sugared function type, and @escaping might be removed if it appears somewhere where we do not mark function types as non-escaping by default. Fixes <https://bugs.swift.org/browse/SR-2053> and <https://bugs.swift.org/browse/SR-2397>.
|
@swift-ci Please test os x |
|
@milseman or @jrose-apple, can you review this for 3.0? |
slavapestov
changed the title
|
Build failed |
|
@tkremenek Do you know if anyone is looking at this? |
|
@slavapestov That failure is supposed to be fixed with swiftlang/swift-package-manager#612. |
|
@swift-ci Please test os x |
| } | ||
|
|
||
| // Hack to apply context-specific @escaping to an AST function type. | ||
| static Type adjustFunctionExtInfo(DeclContext *DC, |
There was a problem hiding this comment.
More specific name here, that is, adjust it to do what?
There was a problem hiding this comment.
Renamed to applyNonEscapingFromContext().
|
The code LGTM, could you otherwise elaborate on what the risk is from this change? |
| if (!ty) ty = resolveType(repr, options); | ||
| if (!ty || ty->is<ErrorType>()) return ty; | ||
| } else if (hasFunctionAttr && fnRepr) { | ||
| for (auto i : FunctionAttrs) |
There was a problem hiding this comment.
What is this part doing?
There was a problem hiding this comment.
It was there before, I think it's just clearing the attributes so that we don't diagnose about them later.
|
Working on a new patch now that adds comments to clarify things. |
|
@swift-ci Please test os x |
|
For posterity: rdar://27918871 |
|
What's the plan for optional function types, too? NTBF for Swift 3? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.