-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[4.0] Diagnose exclusivity violations for noescape closures #10362
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
[4.0] Diagnose exclusivity violations for noescape closures #10362
Conversation
Make helper functions static and avoid defining one except when assertions are enabled.
Relax the static enforcement of exclusive access so that we no longer diagnose on accesses to separate stored structs of the same property: takesInout(&s.f1, &s.f2) // no-warning And perform the analogous relaxation for tuple elements. To do this, track for each begin_access the projection path from that access and record the read and write-like modifications on a per-subpath basis. We still warn if the there are conflicting accesses on subpaths where one is the prefix of another. This commit leaves the diagnostic text in a not-so-good shape since we refer to the DescriptiveDeclKind of the access even describing a subpath. I'll fix that up in a later commit that changes only diagnostic text. https://bugs.swift.org/browse/SR-5119 rdar://problem/31909639
Remove the descriptive decl kind (since with subpaths it is not correct and cannot represent a tuple element) and change "simultaneous" to "overlapping" in order to lower the register slightly and avoid connoting threading. For example, for the following: takesTwoInouts(&x.f, &x.f) the diagnostic will change from "simultaneous accesses to var 'x.f', but modification requires exclusive access; consider copying to a local variable" to "overlapping accesses to 'x.f', but modification requires exclusive access; consider copying to a local variable"
Copy-pasta strikes again!
Move IndexTrieNode from DeadObjectElimination into its own header. I plan to use this data structure when diagnosing static violations of exclusive access.
…le args Add an interprocedural SIL analysis pass that summarizes the accesses that closures make on their @inout_aliasable captures. This will be used to statically enforce exclusivity for calls to functions that take noescape closures. The analysis summarizes the accesses on each argument independently and uses the BottomUpIPAnalysis utility class to iterate to a fixed point when there are cycles in the call graph. For now, the analysis is not stored-property-sensitive -- that will come in a later commit.
…ectionPath IndexTrie is a more light-weight representation and it works well in this case. This requires recovering the represented sequence from an IndexTrieNode, so also add a getParent() method.
swiftlang#10310) Use the AccessSummaryAnalysis to statically enforce exclusive access for noescape closures passed as arguments to functions. We will now diagnose when a function is passed a noescape closure that begins an access on capture when that same capture already has a conflicting access in progress at the time the function is applied. The interprocedural analysis is not yet stored-property sensitive (unlike the intraprocedural analysis), so this will report violations on accesses to separate stored properties of the same struct. rdar://problem/32020710
@swift-ci Please test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved for CCC.
@swift-ci Please test source compatibility |
@swift-ci please clean test |
CCC Information: Explanation: For example, the compiler will now diagnose statically on the following: x.mutatingMethod { This diagnostic a warning in Swift 3.2 and an error in Swift 4 mode. Note: This change does not distinguish separate stored properties Scope of Issue: Radar: rdar://problem/32020710 Risk: This is a medium risk change. It adds new diagnostics (errors in Testing: We've added the usual regression tests and also evaluated on |
This PR enables checking exclusivity violations for no escape closures. For testing purposes it includes all the commits from #10360 plus one additional commit (73d7476) that is the only new content for this PR w/r/t 10360.