Merge pull request #15915 from devincoughlin/exclusivity-reabstraction-thunk-as-error

Author: swift-ci

lib/SILOptimizer/Mandatory/DiagnoseStaticExclusivity.cpp

@@ -770,25 +770,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);
   }
 }
 

test/SILOptimizer/access_enforcement_noescape.swift

@@ -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)
 }
@@ -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)
   }

test/SILOptimizer/exclusivity_static_diagnostics.sil

@@ -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 }
@@ -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 () -> ()
@@ -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 () -> ()

test/SILOptimizer/exclusivity_static_diagnostics.swift

@@ -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}}
   }
@@ -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}}
@@ -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
@@ -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
@@ -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}}
   }
@@ -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()

test/SILOptimizer/exclusivity_static_diagnostics_objc.swift

@@ -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 }
 }