[Exclusivity] Treat warning violations with reabstraction thunks as errors

In Swift 4.1 we fixed a false negative and started looking through
reabstraction thunks to statically find exclusivity violations. To lessen
source impact, we treated these violations as warnings. This commit upgrades
those warnings to errors.

rdar://problem/34669400

Author: devincoughlin

lib/SILOptimizer/Mandatory/DiagnoseStaticExclusivity.cpp

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

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