[Sema] Limit inout capture to @noescape contexts #2354
Conversation
|
@jckarter can you take a look at this? |
| for (auto D : llvm::makeArrayRef(SF.Decls).slice(StartElem)) { | ||
| D->walk(walker); | ||
| } | ||
| } |
There was a problem hiding this comment.
I think we should be able to check this during applySolution instead of walking the entire source file again. When we finishApply on a call that has a local function DeclRef as an argument, we can either check the local function's captures immediately if they've been computed, or else remember whether the function reference was @noescape or not and then diagnose when we computeCaptures later.
|
@swift-ci Please test |
|
Awesome, all tests pass. I can't wait for InOutDeshadowing to bite the dust :-) |
|
@swift-ci Please test |
|
@dduan Thanks for the revision! Linux failure looks unrelated. I'll review the code in a bit. |
|
Reviewing myself: I don't like stashing those states in decl. Would love some alternative spots!
|
| @@ -4598,6 +4600,8 @@ class AbstractFunctionDecl : public ValueDecl, public DeclContext { | |||
|
|
|||
| /// Location of the 'throws' token. | |||
| SourceLoc ThrowsLoc; | |||
| llvm::DenseSet<ApplyExpr *> OccurancesAsEcscapingArg; | |||
There was a problem hiding this comment.
Spelling nitpick: "Occurrence"
|
Looks great, aside from the spelling comment. |
| @@ -4627,6 +4631,18 @@ class AbstractFunctionDecl : public ValueDecl, public DeclContext { | |||
| void setGenericParams(GenericParamList *GenericParams); | |||
|
|
|||
| public: | |||
| llvm::DenseSet<ApplyExpr *> getEscapingArgOccurance() { | |||
| return OccurancesAsEcscapingArg; | |||
|
I agree. Since this info is only needed during Sema, you could move it to a DenseMap in the TypeChecker object. |
|
@swift-ci please test |
|
@jckarter I moved the stash from |
|
@dduan Looks great. Do you mind squashing the commits? |
The following diagnostics have been added: 1. implicit capture of inout arguments by closure literals that may escape ( not mark by @NoEscape) is now invalid. This also includes implicit capture of `self`. 2. nested functions with inout captures cannot be used as arguments not marked @NoEscape. 3. nested function with inout captures cannot be a return value. This change eliminates the need for the shadowing mechanism created for inouts.
|
Thanks! |
|
Should I mark SE-0035 as "implemented"? |
mark it as complete.
[pull] swiftwasm from main
What's in this pull request?
This patch adds limits to inout captures as specified in SE-0035.
The following diagnostics have been added:
mark by @NoEscape) is now invalid. This also includes implicit capture of
self.@NoEscape.
This change eliminates the need for the shadowing mechanism created for inout.
The shadow copy and InOutDeshadowing SIL pass has not been eliminated yet. It's WIP and should come in a later PR.
Resolved bug number: (SR-807)
Before merging this pull request to apple/swift repository:
Triggering Swift CI
The swift-ci is triggered by writing a comment on this PR addressed to the GitHub user @swift-ci. Different tests will run depending on the specific comment that you use. The currently available comments are:
Smoke Testing
Validation Testing
Note: Only members of the Apple organization can trigger swift-ci.