-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[4.0] [Exclusivity] Relax intra-procedural diagnostics on separate stored structs #10360
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] [Exclusivity] Relax intra-procedural diagnostics on separate stored structs #10360
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.
@swift-ci Please test |
@swift-ci Please test source compatibility |
Note: The failures on the compatibility suite are unrelated to this PR. |
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.
Looks good for CCC.
Thanks!
CCC Information: swap(&s.f, &s.g) where s is a struct with stored properties f and g. Scope: This is a persistent source of false positives in the static enforcement. Radar: rdar://problem/31909639 Risk: This is a relatively low risk change. It causes the compiler to warn in Testing: We've added the usual regression tests and also confirmed that the |
This is a series of commits that relaxes the intra-procedural static enforcement of exclusive access so that we no longer diagnose on accesses to separate stored structs of the same property.
https://bugs.swift.org/browse/SR-5119
rdar://problem/31909639
This PR includes the AccessSummaryAnalysis, although the only thing it is used for in this PR is the shared IndexTrie to represent projection paths. This PR doesn't actually run any of the inter procedural closure analysis. That will be CCC'd to swift-4.0 in a different in a different PR.