Skip to content

[Exclusivity] Treat warning violations with reabstraction thunks as e… #15915

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Apr 16, 2018
Merged

[Exclusivity] Treat warning violations with reabstraction thunks as e… #15915

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 1 addition & 13 deletions lib/SILOptimizer/Mandatory/DiagnoseStaticExclusivity.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -960,25 +960,13 @@ static void checkForViolationsInNoEscapeClosureArguments(
if (!Callee || Callee->empty())
continue;

// For source compatibility reasons, treat conflicts found by
// looking through reabstraction thunks as warnings. A future compiler
// will upgrade these to errors;
DiagnoseAsWarning |= result.isReabstructionThunk;

// For source compatibility reasons, treat conflicts found by
// looking through noescape blocks as warnings. A future compiler
// will upgrade these to errors.
DiagnoseAsWarning |=
(getSILFunctionTypeForValue(Argument)->getRepresentation()
== SILFunctionTypeRepresentation::Block);

// Check the closure's captures, which are a suffix of the closure's
// parameters.
unsigned StartIndex =
Callee->getArguments().size() - result.PAI->getNumArguments();
checkForViolationWithCall(Accesses, Callee, StartIndex,
result.PAI->getArguments(), ASA,
DiagnoseAsWarning, ConflictingAccesses);
/*DiagnoseAsWarning=*/false, ConflictingAccesses);
}
}

Expand Down
4 changes: 2 additions & 2 deletions test/SILOptimizer/access_enforcement_noescape.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -536,7 +536,7 @@ func readBlockWriteInout() {
var x = 3
// Around the call: [modify] [static]
// Inside closure: [read] [static]
// expected-warning@+2{{overlapping accesses to 'x', but modification requires exclusive access; consider copying to a local variable}}
// expected-error@+2{{overlapping accesses to 'x', but modification requires exclusive access; consider copying to a local variable}}
// expected-note@+1{{conflicting access is here}}
doBlockInout({ _ = x }, &x)
}
Expand All @@ -561,7 +561,7 @@ func readBlockWriteInout() {
func noEscapeBlock() {
var x = 3
doOne {
// expected-warning@+2{{overlapping accesses to 'x', but modification requires exclusive access; consider copying to a local variable}}
// expected-error@+2{{overlapping accesses to 'x', but modification requires exclusive access; consider copying to a local variable}}
// expected-note@+1{{conflicting access is here}}
doBlockInout({ _ = x }, &x)
}
Expand Down
6 changes: 3 additions & 3 deletions test/SILOptimizer/exclusivity_static_diagnostics.sil
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,7 @@ bb0(%0 : $Int):
%8 = function_ref @thunkForClosureWithConcreteReturn : $@convention(thin) (Int, @noescape @callee_owned (Int) -> Int) -> @out Int
%9 = partial_apply %8(%7) : $@convention(thin) (Int, @noescape @callee_owned (Int) -> Int) -> @out Int
%10 = convert_escape_to_noescape %9 : $@callee_owned (Int) -> @out Int to $@noescape @callee_owned (Int) -> @out Int
%11 = begin_access [modify] [unknown] %3 : $*Int // expected-warning {{overlapping accesses, but modification requires exclusive access; consider copying to a local variable}}
%11 = begin_access [modify] [unknown] %3 : $*Int // expected-error {{overlapping accesses, but modification requires exclusive access; consider copying to a local variable}}
%12 = apply %4<Int>(%11, %10) : $@convention(thin) <T_0> (@inout Int, @noescape @callee_owned (Int) -> @out T_0) -> ()
end_access %11: $*Int
destroy_value %2 : ${ var Int }
Expand All @@ -698,7 +698,7 @@ bb0(%0 : $Int):
%10 = function_ref @thunkForCalleeGuaranteed : $@convention(c) (@inout_aliasable @block_storage @noescape @callee_guaranteed () -> ()) -> ()
%11 = init_block_storage_header %7 : $*@block_storage @noescape @callee_guaranteed () -> (), invoke %10 : $@convention(c) (@inout_aliasable @block_storage @noescape @callee_guaranteed () -> ()) -> (), type $@convention(block) @noescape () -> ()
%12 = copy_block %11 : $@convention(block) @noescape () -> ()
%13 = begin_access [modify] [unknown] %3 : $*Int // expected-warning {{overlapping accesses, but modification requires exclusive access; consider copying to a local variable}}
%13 = begin_access [modify] [unknown] %3 : $*Int // expected-error {{overlapping accesses, but modification requires exclusive access; consider copying to a local variable}}
%14 = apply %4(%13, %12) : $@convention(thin) (@inout Int, @owned @convention(block) @noescape () -> ()) -> ()
end_access %13 : $*Int
destroy_addr %8 : $*@callee_guaranteed @noescape () -> ()
Expand All @@ -725,7 +725,7 @@ bb0(%0 : $Int):
%11 = init_block_storage_header %7 : $*@block_storage @noescape @callee_guaranteed () -> (), invoke %10 : $@convention(c) (@inout_aliasable @block_storage @noescape @callee_guaranteed () -> ()) -> (), type $@convention(block) @noescape () -> ()
%12 = copy_block %11 : $@convention(block) @noescape () -> ()
%13 = enum $Optional<@convention(block) @noescape () -> ()>, #Optional.some!enumelt.1, %12 : $@convention(block) @noescape () -> ()
%14 = begin_access [modify] [unknown] %3 : $*Int // expected-warning {{overlapping accesses, but modification requires exclusive access; consider copying to a local variable}}
%14 = begin_access [modify] [unknown] %3 : $*Int // expected-error {{overlapping accesses, but modification requires exclusive access; consider copying to a local variable}}
%15 = apply %4(%14, %13) : $@convention(thin) (@inout Int, @owned Optional<@convention(block) @noescape () -> ()>) -> ()
end_access %14 : $*Int
destroy_addr %8 : $*@callee_guaranteed @noescape () -> ()
Expand Down
44 changes: 39 additions & 5 deletions test/SILOptimizer/exclusivity_static_diagnostics.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ struct StructWithTwoStoredProp {
// Take an unsafe pointer to a stored property while accessing another stored property.
func violationWithUnsafePointer(_ s: inout StructWithTwoStoredProp) {
withUnsafePointer(to: &s.f1) { (ptr) in
// expected-warning@-1 {{overlapping accesses to 's.f1', but modification requires exclusive access; consider copying to a local variable}}
// expected-error@-1 {{overlapping accesses to 's.f1', but modification requires exclusive access; consider copying to a local variable}}
_ = s.f1
// expected-note@-1 {{conflicting access is here}}
}
Expand Down Expand Up @@ -196,6 +196,18 @@ func callsTakesInoutAndNoEscapeClosure() {
}
}

func inoutReadWriteInout(x: inout Int) {
// expected-error@+2{{overlapping accesses to 'x', but modification requires exclusive access; consider copying to a local variable}}
// expected-note@+1{{conflicting access is here}}
takesInoutAndNoEscapeClosure(&x, { _ = x })
}

func inoutWriteWriteInout(x: inout Int) {
// expected-error@+2{{overlapping accesses to 'x', but modification requires exclusive access; consider copying to a local variable}}
// expected-note@+1{{conflicting access is here}}
takesInoutAndNoEscapeClosure(&x, { x = 42 })
}

func callsTakesInoutAndNoEscapeClosureWithRead() {
var local = 5
takesInoutAndNoEscapeClosure(&local) { // expected-error {{overlapping accesses to 'local', but modification requires exclusive access; consider copying to a local variable}}
Expand Down Expand Up @@ -338,7 +350,7 @@ func testReabstractionThunk(p1: inout ParameterizedStruct<Int>,
// This tests that we still detect access violations for closures passed
// using a reabstraction thunk.
p1.takesFunctionWithGenericReturnType { _ in
// expected-warning@-1 {{overlapping accesses to 'p1', but modification requires exclusive access; consider copying to a local variable}}
// expected-error@-1 {{overlapping accesses to 'p1', but modification requires exclusive access; consider copying to a local variable}}
p2 = p1
// expected-note@-1 {{conflicting access is here}}
return 3
Expand All @@ -359,12 +371,21 @@ func takesEscapingBlockClosure
func testCallNoEscapeBlockClosure() {
var i = 7
takesNoEscapeBlockClosure(&i) {
// expected-warning@-1 {{overlapping accesses to 'i', but modification requires exclusive access; consider copying to a local variable}}
// expected-error@-1 {{overlapping accesses to 'i', but modification requires exclusive access; consider copying to a local variable}}
i = 7
// expected-note@-1 {{conflicting access is here}}
}
}

func testCallNoEscapeBlockClosureRead() {
var i = 7
takesNoEscapeBlockClosure(&i) {
// expected-error@-1 {{overlapping accesses to 'i', but modification requires exclusive access; consider copying to a local variable}}
_ = i
// expected-note@-1 {{conflicting access is here}}
}
}

func testCallEscapingBlockClosure() {
var i = 7
takesEscapingBlockClosure(&i) { // no-warning
Expand All @@ -388,7 +409,7 @@ func takesInoutAndClosureWithGenericArg<T>(_ p: inout Int, _ c: (T) -> Int) { }
func callsTakesInoutAndClosureWithGenericArg() {
var i = 7
takesInoutAndClosureWithGenericArg(&i) { (p: Int) in
// expected-warning@-1 {{overlapping accesses to 'i', but modification requires exclusive access; consider copying to a local variable}}
// expected-error@-1 {{overlapping accesses to 'i', but modification requires exclusive access; consider copying to a local variable}}
return i + p
// expected-note@-1 {{conflicting access is here}}
}
Expand All @@ -399,12 +420,25 @@ func callsTakesInoutAndClosureTakingNonOptionalWithClosureTakingOptional() {
var i = 7
// Test for the thunk converting an (Int?) -> () to an (Int) -> ()
takesInoutAndClosureTakingNonOptional(&i) { (p: Int?) in
// expected-warning@-1 {{overlapping accesses to 'i', but modification requires exclusive access; consider copying to a local variable}}
// expected-error@-1 {{overlapping accesses to 'i', but modification requires exclusive access; consider copying to a local variable}}
i = 8
// expected-note@-1 {{conflicting access is here}}
}
}

// Helper.
func doOne(_ f: () -> ()) {
f()
}

func noEscapeBlock() {
var x = 3
doOne {
// expected-error@+2{{overlapping accesses to 'x', but modification requires exclusive access; consider copying to a local variable}}
// expected-note@+1{{conflicting access is here}}
takesInoutAndNoEscapeClosure(&x, { _ = x })
}
}

func inoutSeparateStructStoredProperties() {
var s = StructWithTwoStoredProp()
Expand Down
2 changes: 1 addition & 1 deletion test/SILOptimizer/exclusivity_static_diagnostics_objc.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class SomeClass {

func testOptionalImport() {
var x = 0
// expected-warning@+2{{overlapping accesses to 'x', but modification requires exclusive access; consider copying to a local variable}}
// expected-error@+2{{overlapping accesses to 'x', but modification requires exclusive access; consider copying to a local variable}}
// expected-note@+1{{conflicting access is here}}
SomeObjCInterface.perform(&x) { x += 1 }
}