Merge pull request swiftlang#9373 from devincoughlin/static-enforcement-error-in-swift-4-mode

Author: swift-ci

include/swift/AST/DiagnosticsSIL.def

@@ -84,14 +84,22 @@ ERROR(inout_argument_alias,none,
 NOTE(previous_inout_alias,none,
       "previous aliasing argument", ())
 
-// This is temporarily a warning during staging to make it easier to evaluate.
-// The intent is to change it to an error before turning it on by default.
-WARNING(exclusivity_access_required,none,
+WARNING(exclusivity_access_required_swift3,none,
         "simultaneous accesses to %0 %1; "
         "%select{initialization|read|modification|deinitialization}2 requires "
         "exclusive access", (DescriptiveDeclKind, Identifier, unsigned))
 
-WARNING(exclusivity_access_required_unknown_decl,none,
+ERROR(exclusivity_access_required,none,
+      "simultaneous accesses to %0 %1; "
+      "%select{initialization|read|modification|deinitialization}2 requires "
+      "exclusive access", (DescriptiveDeclKind, Identifier, unsigned))
+
+ERROR(exclusivity_access_required_unknown_decl,none,
+        "simultaneous accesses; "
+        "%select{initialization|read|modification|deinitialization}0 requires "
+        "exclusive access", (unsigned))
+
+WARNING(exclusivity_access_required_unknown_decl_swift3,none,
         "simultaneous accesses; "
         "%select{initialization|read|modification|deinitialization}0 requires "
         "exclusive access", (unsigned))

lib/SILOptimizer/Mandatory/DiagnoseStaticExclusivity.cpp

@@ -230,7 +230,8 @@ class AccessInfo {
 
 /// Indicates whether a 'begin_access' requires exclusive access
 /// or allows shared access. This needs to be kept in sync with
-/// diag::exclusivity_access_required and diag::exclusivity_conflicting_access.
+/// diag::exclusivity_access_required, exclusivity_access_required_swift3,
+/// and diag::exclusivity_conflicting_access.
 enum class ExclusiveOrShared_t : unsigned {
   ExclusiveAccess = 0,
   SharedAccess = 1
@@ -382,15 +383,22 @@ static void diagnoseExclusivityViolation(const AccessedStorage &Storage,
     AccessForMainDiagnostic->getLoc().getSourceRange();
 
   if (const ValueDecl *VD = Storage.getStorageDecl()) {
+    // We have a declaration, so mention the identifier in the diagnostic.
+    auto DiagnosticID = (Ctx.LangOpts.isSwiftVersion3() ?
+                         diag::exclusivity_access_required_swift3 :
+                         diag::exclusivity_access_required);
     diagnose(Ctx, AccessForMainDiagnostic->getLoc().getSourceLoc(),
-             diag::exclusivity_access_required,
+             DiagnosticID,
              VD->getDescriptiveKind(),
              VD->getName(),
              static_cast<unsigned>(AccessForMainDiagnostic->getAccessKind()))
         .highlight(rangeForMain);
   } else {
+    auto DiagnosticID = (Ctx.LangOpts.isSwiftVersion3() ?
+                         diag::exclusivity_access_required_unknown_decl_swift3 :
+                         diag::exclusivity_access_required_unknown_decl);
     diagnose(Ctx, AccessForMainDiagnostic->getLoc().getSourceLoc(),
-             diag::exclusivity_access_required_unknown_decl,
+             DiagnosticID,
              static_cast<unsigned>(AccessForMainDiagnostic->getAccessKind()))
         .highlight(rangeForMain);
   }

test/SILOptimizer/exclusivity_static_diagnostics.sil

@@ -38,7 +38,7 @@ bb0(%0 : $Int):
   %3 = project_box %2 : ${ var Int }, 0
   store %0 to [trivial] %3 : $*Int
   %4 = function_ref @takesTwoInouts : $@convention(thin) (@inout Int, @inout Int) -> ()
-  %5 = begin_access [modify] [unknown] %3 : $*Int  // expected-warning {{simultaneous accesses; modification requires exclusive access}}
+  %5 = begin_access [modify] [unknown] %3 : $*Int  // expected-error {{simultaneous accesses; modification requires exclusive access}}
   %6 = begin_access [modify] [unknown] %3 : $*Int  // expected-note {{conflicting access is here}}
   %7 = apply %4(%5, %6) : $@convention(thin) (@inout Int, @inout Int) -> ()
   end_access %6 : $*Int
@@ -55,7 +55,7 @@ bb0(%0 : $Int):
   %3 = project_box %2 : ${ var Int }, 0
   store %0 to [trivial] %3 : $*Int
   %4 = function_ref @takesTwoInouts : $@convention(thin) (@inout Int, @inout Int) -> ()
-  %5 = begin_access [modify] [unknown] %3 : $*Int  // expected-warning {{simultaneous accesses; modification requires exclusive access}}
+  %5 = begin_access [modify] [unknown] %3 : $*Int  // expected-error {{simultaneous accesses; modification requires exclusive access}}
   %6 = begin_access [modify] [unknown] %5 : $*Int
   %7 = begin_access [modify] [unknown] %3 : $*Int  // expected-note {{conflicting access is here}}
   %8 = apply %4(%5, %6) : $@convention(thin) (@inout Int, @inout Int) -> ()
@@ -147,7 +147,7 @@ bb0(%0 : $Int, %1 : $Builtin.Int1):
   br bb1
 bb1:
   // Make sure we don't diagnose twice.
-  %4 = begin_access [modify] [unknown] %3 : $*Int // expected-warning {{simultaneous accesses; modification requires exclusive access}}
+  %4 = begin_access [modify] [unknown] %3 : $*Int // expected-error {{simultaneous accesses; modification requires exclusive access}}
   %5 = begin_access [modify] [unknown] %3 : $*Int // expected-note {{conflicting access is here}}
   end_access %5: $*Int
   end_access %4: $*Int
@@ -199,7 +199,7 @@ bb0(%0 : $Int):
   %2 = project_box %1 : ${ var Int }, 0
   store %0 to [trivial] %2 : $*Int
   %4 = begin_access [read] [unknown] %2 : $*Int // expected-note {{conflicting access is here}}
-  %5 = begin_access [modify] [unknown] %2 : $*Int // expected-warning {{simultaneous accesses; modification requires exclusive access}}
+  %5 = begin_access [modify] [unknown] %2 : $*Int // expected-error {{simultaneous accesses; modification requires exclusive access}}
   end_access %5 : $*Int
   end_access %4: $*Int
   destroy_value %1 : ${ var Int }
@@ -213,7 +213,7 @@ bb0(%0 : $Int):
   %1 = alloc_box ${ var Int }
   %2 = project_box %1 : ${ var Int }, 0
   store %0 to [trivial] %2 : $*Int
-  %4 = begin_access [modify] [unknown] %2 : $*Int // expected-warning {{simultaneous accesses; modification requires exclusive access}}
+  %4 = begin_access [modify] [unknown] %2 : $*Int // expected-error {{simultaneous accesses; modification requires exclusive access}}
   %5 = begin_access [read] [unknown] %2 : $*Int // expected-note {{conflicting access is here}}
   end_access %5 : $*Int
   end_access %4: $*Int
@@ -232,7 +232,7 @@ sil hidden @classStoredProperty : $@convention(thin) (ClassWithStoredProperty) -
 bb0(%0 : $ClassWithStoredProperty):
   %1 = ref_element_addr %0 : $ClassWithStoredProperty, #ClassWithStoredProperty.f
 
-  // expected-warning@+1{{simultaneous accesses to var 'f'; modification requires exclusive access}}
+  // expected-error@+1{{simultaneous accesses to var 'f'; modification requires exclusive access}}
   %2 = begin_access [modify] [dynamic] %1 : $*Int
   %3 = ref_element_addr %0 : $ClassWithStoredProperty, #ClassWithStoredProperty.f
 
@@ -252,7 +252,7 @@ bb0(%0 : $ClassWithStoredProperty):
   %2 = begin_borrow %0 : $ClassWithStoredProperty
   %3 = ref_element_addr %1 : $ClassWithStoredProperty, #ClassWithStoredProperty.f
 
-  // expected-warning@+1{{simultaneous accesses to var 'f'; modification requires exclusive access}}
+  // expected-error@+1{{simultaneous accesses to var 'f'; modification requires exclusive access}}
   %4 = begin_access [modify] [dynamic] %3 : $*Int
   %5 = ref_element_addr %2 : $ClassWithStoredProperty, #ClassWithStoredProperty.f
 
@@ -279,7 +279,7 @@ bb0(%0 : $Int):
   store %0 to [trivial] %3 : $*Int
   %4 = copy_value %2 : ${ var Int }
   %5 = project_box %4 : ${ var Int }, 0
-  %6 = begin_access [modify] [unknown] %3 : $*Int  // expected-warning {{simultaneous accesses; modification requires exclusive access}}
+  %6 = begin_access [modify] [unknown] %3 : $*Int  // expected-error {{simultaneous accesses; modification requires exclusive access}}
   %7 = begin_access [modify] [unknown] %5 : $*Int  // expected-note {{conflicting access is here}}
   end_access %7 : $*Int
   end_access %6: $*Int
@@ -297,7 +297,7 @@ bb0(%0 : $Int):
   %3 = project_box %2 : ${ var Int }, 0
   store %0 to [trivial] %3 : $*Int
   %4 = project_box %2 : ${ var Int }, 0
-  %5 = begin_access [modify] [unknown] %3 : $*Int  // expected-warning {{simultaneous accesses; modification requires exclusive access}}
+  %5 = begin_access [modify] [unknown] %3 : $*Int  // expected-error {{simultaneous accesses; modification requires exclusive access}}
   %6 = begin_access [modify] [unknown] %4 : $*Int  // expected-note {{conflicting access is here}}
   end_access %6 : $*Int
   end_access %5: $*Int
@@ -315,7 +315,7 @@ sil hidden @modifySameGlobal : $@convention(thin) (Int) -> () {
 bb0(%0 : $Int):
   %1 = global_addr @global1 :$*Int
   %2 = global_addr @global1 :$*Int
-  %3 = begin_access [modify] [unknown] %1 : $*Int  // expected-warning {{simultaneous accesses; modification requires exclusive access}}
+  %3 = begin_access [modify] [unknown] %1 : $*Int  // expected-error {{simultaneous accesses; modification requires exclusive access}}
   %4 = begin_access [modify] [unknown] %2 : $*Int  // expected-note {{conflicting access is here}}
   end_access %4 : $*Int
   end_access %3: $*Int
@@ -348,7 +348,7 @@ bb0(%0 : $Int):
   store %0 to [trivial] %3 : $*Int
   %4 = function_ref @takesTwoInouts : $@convention(thin) (@inout Int, @inout Int) -> ()
   %5 = begin_access [read] [unknown] %3 : $*Int  // expected-note {{conflicting access is here}}
-  %6 = begin_access [modify] [unknown] %3 : $*Int  // expected-warning {{simultaneous accesses; modification requires exclusive access}}
+  %6 = begin_access [modify] [unknown] %3 : $*Int  // expected-error {{simultaneous accesses; modification requires exclusive access}}
   %7 = begin_access [read] [unknown] %3 : $*Int // no-error
   %8 = apply %4(%5, %6) : $@convention(thin) (@inout Int, @inout Int) -> ()
   end_access %7 : $*Int
@@ -368,7 +368,7 @@ bb0(%0 : $Int):
   %3 = project_box %2 : ${ var Int }, 0
   store %0 to [trivial] %3 : $*Int
   %4 = function_ref @takesTwoInouts : $@convention(thin) (@inout Int, @inout Int) -> ()
-  %5 = begin_access [modify] [unknown] %3 : $*Int  // expected-warning {{simultaneous accesses; modification requires exclusive access}}
+  %5 = begin_access [modify] [unknown] %3 : $*Int  // expected-error {{simultaneous accesses; modification requires exclusive access}}
   %6 = begin_access [modify] [unknown] %3 : $*Int  // expected-note {{conflicting access is here}}
   %7 = begin_access [modify] [unknown] %3 : $*Int  // no-error
   %8 = apply %4(%5, %6) : $@convention(thin) (@inout Int, @inout Int) -> ()
@@ -392,7 +392,7 @@ bb0(%0 : $Int):
   %4 = function_ref @takesTwoInouts : $@convention(thin) (@inout Int, @inout Int) -> ()
   %5 = begin_access [modify] [unknown] %3 : $*Int  // no-note
   end_access %5 : $*Int
-  %6 = begin_access [modify] [unknown] %3 : $*Int  // expected-warning {{simultaneous accesses; modification requires exclusive access}}
+  %6 = begin_access [modify] [unknown] %3 : $*Int  // expected-error {{simultaneous accesses; modification requires exclusive access}}
   %7 = begin_access [modify] [unknown] %3 : $*Int  // expected-note {{conflicting access is here}}
   %8 = apply %4(%5, %6) : $@convention(thin) (@inout Int, @inout Int) -> ()
   end_access %7 : $*Int

test/SILOptimizer/exclusivity_static_diagnostics.swift

@@ -1,4 +1,4 @@
-// RUN: %target-swift-frontend -enforce-exclusivity=checked -emit-sil -primary-file %s -o /dev/null -verify
+// RUN: %target-swift-frontend -enforce-exclusivity=checked -swift-version 4 -emit-sil -primary-file %s -o /dev/null -verify
 
 import Swift
 
@@ -7,25 +7,27 @@ func takesTwoInouts<T>(_ p1: inout T, _ p2: inout T) { }
 func simpleInoutDiagnostic() {
   var i = 7
 
+  // FIXME: This diagnostic should be removed if static enforcement is
+  // turned on by default.
   // expected-error@+4{{inout arguments are not allowed to alias each other}}
   // expected-note@+3{{previous aliasing argument}}
-  // expected-warning@+2{{simultaneous accesses to var 'i'; modification requires exclusive access}}
+  // expected-error@+2{{simultaneous accesses to var 'i'; modification requires exclusive access}}
   // expected-note@+1{{conflicting access is here}}
   takesTwoInouts(&i, &i)
 }
 
 func inoutOnInoutParameter(p: inout Int) {
   // expected-error@+4{{inout arguments are not allowed to alias each other}}
   // expected-note@+3{{previous aliasing argument}}
-  // expected-warning@+2{{simultaneous accesses to parameter 'p'; modification requires exclusive access}}
+  // expected-error@+2{{simultaneous accesses to parameter 'p'; modification requires exclusive access}}
   // expected-note@+1{{conflicting access is here}}
   takesTwoInouts(&p, &p)
 }
 
 func swapNoSuppression(_ i: Int, _ j: Int) {
   var a: [Int] = [1, 2, 3]
 
-  // expected-warning@+2{{simultaneous accesses to var 'a'; modification requires exclusive access}}
+  // expected-error@+2{{simultaneous accesses to var 'a'; modification requires exclusive access}}
   // expected-note@+1{{conflicting access is here}}
   swap(&a[i], &a[j]) // no-warning
 }
@@ -40,21 +42,21 @@ struct StructWithMutatingMethodThatTakesSelfInout {
   mutating func callMutatingMethodThatTakesSelfInout() {
     // expected-error@+4{{inout arguments are not allowed to alias each other}}
     // expected-note@+3{{previous aliasing argument}}
-    // expected-warning@+2{{simultaneous accesses to parameter 'self'; modification requires exclusive access}}
+    // expected-error@+2{{simultaneous accesses to parameter 'self'; modification requires exclusive access}}
     // expected-note@+1{{conflicting access is here}}
     mutate(&self)
   }
 
   mutating func callMutatingMethodThatTakesSelfStoredPropInout() {
-    // expected-warning@+2{{simultaneous accesses to parameter 'self'; modification requires exclusive access}}
+    // expected-error@+2{{simultaneous accesses to parameter 'self'; modification requires exclusive access}}
     // expected-note@+1{{conflicting access is here}}
     mutate(&self.f)
   }
 }
 
 var globalStruct1 = StructWithMutatingMethodThatTakesSelfInout()
 func callMutatingMethodThatTakesGlobalStoredPropInout() {
-  // expected-warning@+2{{simultaneous accesses to var 'globalStruct1'; modification requires exclusive access}}
+  // expected-error@+2{{simultaneous accesses to var 'globalStruct1'; modification requires exclusive access}}
   // expected-note@+1{{conflicting access is here}}
   globalStruct1.mutate(&globalStruct1.f)
 }
@@ -66,13 +68,13 @@ class ClassWithFinalStoredProp {
   func callMutatingMethodThatTakesClassStoredPropInout() {
     s1.mutate(&s2.f) // no-warning
 
-    // expected-warning@+2{{simultaneous accesses to var 's1'; modification requires exclusive access}}
+    // expected-error@+2{{simultaneous accesses to var 's1'; modification requires exclusive access}}
     // expected-note@+1{{conflicting access is here}}
     s1.mutate(&s1.f)
 
     let local1 = self
 
-    // expected-warning@+2{{simultaneous accesses to var 's1'; modification requires exclusive access}}
+    // expected-error@+2{{simultaneous accesses to var 's1'; modification requires exclusive access}}
     // expected-note@+1{{conflicting access is here}}
     local1.s1.mutate(&local1.s1.f)
   }
@@ -82,7 +84,7 @@ func violationWithGenericType<T>(_ p: T) {
   var local = p
   // expected-error@+4{{inout arguments are not allowed to alias each other}}
   // expected-note@+3{{previous aliasing argument}}
-  // expected-warning@+2{{simultaneous accesses to var 'local'; modification requires exclusive access}}
+  // expected-error@+2{{simultaneous accesses to var 'local'; modification requires exclusive access}}
   // expected-note@+1{{conflicting access is here}}
   takesTwoInouts(&local, &local)
 }

test/SILOptimizer/exclusivity_static_diagnostics_swift3.swift

@@ -0,0 +1,30 @@
+// RUN: %target-swift-frontend -enforce-exclusivity=checked -swift-version 3 -emit-sil -primary-file %s -o /dev/null -verify
+
+import Swift
+
+// In Swift 3 compatibility mode, diagnostics for exclusive accesses are
+// warnings not errors.
+
+func takesTwoInouts<T>(_ p1: inout T, _ p2: inout T) { }
+
+func simpleInoutDiagnostic() {
+  var i = 7
+
+  // expected-error@+4{{inout arguments are not allowed to alias each other}}
+  // expected-note@+3{{previous aliasing argument}}
+  // expected-warning@+2{{simultaneous accesses to var 'i'; modification requires exclusive access}}
+  // expected-note@+1{{conflicting access is here}}
+  takesTwoInouts(&i, &i)
+}
+
+struct X {
+  var f = 12
+}
+
+func diagnoseOnSameField() {
+  var x = X()
+
+  // expected-warning@+2{{simultaneous accesses to var 'x'; modification requires exclusive access}}
+  // expected-note@+1{{conflicting access is here}}
+  takesTwoInouts(&x.f, &x.f)
+}