[4.0] [Exclusivity] Relax closure enforcement on separate stored properties… #10860
Conversation
…swiftlang#10789) Make the static enforcement of accesses in noescape closures stored-property sensitive. This will relax the existing enforcement so that the following is not diagnosed: struct MyStruct { var x = X() var y = Y() mutating func foo() { x.mutatesAndTakesClosure() { _ = y.read() // no-warning } } } To do this, update the access summary analysis to summarize accesses to subpaths of a capture. rdar://problem/32987932
|
@atrick Would you mind reviewing for CCC inclusion into swift-4.0-branch? |
devincoughlin
changed the title
|
@swift-ci Please test |
There was a problem hiding this comment.
🥇 CCC reviewed for 4.0.
@devincoughlin This looks great! Very glad it's going into 4.0.
The three "TODO" comments do not need to be addressed in 4.0. They are for future follow-up.
| bool changed = false; | ||
|
|
||
| const SubAccessMap &otherAccesses = other.SubAccesses; | ||
| for (auto it = otherAccesses.begin(), e = otherAccesses.end(); it != e; |
There was a problem hiding this comment.
TODO: Add a comment explaining why nondeterministic iteration order yields deterministic results (the merge order can affect the DenseMap's internal state, but not it's semantics--iterating over the final result is nondeterministic anyway).
| Optional<RecordedAccess> BestInProgressAccess; | ||
| Optional<RecordedAccess> BestArgAccess; | ||
|
|
||
| for (const auto &MapPair : AS.getSubAccesses()) { |
There was a problem hiding this comment.
TODO: Again, this is iterating over a DenseMap. Since only the "best" conflict is selected, it probably doesn't matter. But this assumptions needs to be clearly commented.
| const auto &SubAccesses = AS.getSubAccesses(); | ||
|
|
||
| // Is the capture accessed in the callee? | ||
| if (SubAccesses.size() == 0) |
There was a problem hiding this comment.
TODO: could be if (AS.getSubAccesses().empty())
|
CCC Information: Scope: This is not a source-breaking change. It results in fewer diagnostics being Radar (and possibly SR Issue): rdar://problem/32987932 Testing: I've extended the unit tests and run the change on the Source Compatibility |
|
Aproved by @bob-wilson for CCC. |
|
@swift-ci Please smoke test and merge |
… (#10789)
Make the static enforcement of accesses in noescape closures stored-property
sensitive. This will relax the existing enforcement so that the following is
not diagnosed:
struct MyStruct {
var x = X()
var y = Y()
mutating
func foo() {
x.mutatesAndTakesClosure() {
_ = y.read() // no-warning
}
}
}
To do this, update the access summary analysis to summarize accesses to
subpaths of a capture.
rdar://problem/32987932