[stdlib] Update Array.subscript to use _modify (#19154)

airspeedswift · web-flow · commit b4a23adcf060 · 2018-09-07T10:36:58.000-07:00
* Switch Array to use subscript _modify

* Add _modify to ContiguousArray

* XFAIL linux failing test
diff --git a/stdlib/public/core/Array.swift b/stdlib/public/core/Array.swift
@@ -570,10 +570,11 @@ extension Array: RandomAccessCollection, MutableCollection {
         index, wasNativeTypeChecked: wasNativeTypeChecked,
         matchingSubscriptCheck: token)
     }
-    mutableAddressWithNativeOwner {
+    _modify {
       _makeMutableAndUnique() // makes the array native, too
       _checkSubscript_native(index)
-      return (_getElementAddress(index), _getOwner_native())
+      let address = _buffer.subscriptBaseAddress + index
+      yield &address.pointee
     }
   }
 
@@ -646,31 +647,6 @@ extension Array {
     return _buffer.capacity
   }
 
-  /// - Precondition: The array has a native buffer.
-  @inlinable
-  @_semantics("array.owner")
-  internal func _getOwnerWithSemanticLabel_native() -> Builtin.NativeObject {
-    return Builtin.unsafeCastToNativeObject(_buffer.nativeOwner)
-  }
-
-  /// - Precondition: The array has a native buffer.
-  @inlinable
-  @inline(__always)
-  internal func _getOwner_native() -> Builtin.NativeObject {
-#if _runtime(_ObjC)
-    if _isClassOrObjCExistential(Element.self) {
-      // We are hiding the access to '_buffer.owner' behind
-      // _getOwner() to help the compiler hoist uniqueness checks in
-      // the case of class or Objective-C existential typed array
-      // elements.
-      return _getOwnerWithSemanticLabel_native()
-    }
-#endif
-    // In the value typed case the extra call to
-    // _getOwnerWithSemanticLabel_native hinders optimization.
-    return Builtin.unsafeCastToNativeObject(_buffer.owner)
-  }
-
   @inlinable
   @_semantics("array.make_mutable")
   internal mutating func _makeMutableAndUnique() {
diff --git a/stdlib/public/core/ContiguousArray.swift b/stdlib/public/core/ContiguousArray.swift
@@ -286,24 +286,14 @@ extension ContiguousArray: RandomAccessCollection, MutableCollection {
   @inlinable
   public subscript(index: Int) -> Element {
     get {
-      // This call may be hoisted or eliminated by the optimizer.  If
-      // there is an inout violation, this value may be stale so needs to be
-      // checked again below.
-      let wasNativeTypeChecked = _hoistableIsNativeTypeChecked()
-
-      // Make sure the index is in range and wasNativeTypeChecked is
-      // still valid.
-      let token = _checkSubscript(
-        index, wasNativeTypeChecked: wasNativeTypeChecked)
-
-      return _getElement(
-        index, wasNativeTypeChecked: wasNativeTypeChecked,
-        matchingSubscriptCheck: token)
+      _checkSubscript_native(index)
+      return _buffer.getElement(index)
     }
-    mutableAddressWithNativeOwner {
-      _makeMutableAndUnique() // makes the array native, too
+    _modify {
+      _makeMutableAndUnique()
       _checkSubscript_native(index)
-      return (_getElementAddress(index), _getOwner_native())
+      let address = _buffer.subscriptBaseAddress + index
+      yield &address.pointee
     }
   }
 
@@ -353,17 +343,6 @@ extension ContiguousArray: RandomAccessCollection, MutableCollection {
 
 //===--- private helpers---------------------------------------------------===//
 extension ContiguousArray {
-  /// Returns `true` if the array is native and does not need a deferred
-  /// type check.  May be hoisted by the optimizer, which means its
-  /// results may be stale by the time they are used if there is an
-  /// inout violation in user code.
-  @inlinable
-  @_semantics("array.props.isNativeTypeChecked")
-  public // @testable
-  func _hoistableIsNativeTypeChecked() -> Bool {
-   return _buffer.arrayPropertyIsNativeTypeChecked
-  }
-
   @inlinable
   @_semantics("array.get_count")
   internal func _getCount() -> Int {
@@ -376,31 +355,6 @@ extension ContiguousArray {
     return _buffer.capacity
   }
 
-  /// - Precondition: The array has a native buffer.
-  @inlinable
-  @_semantics("array.owner")
-  internal func _getOwnerWithSemanticLabel_native() -> Builtin.NativeObject {
-    return Builtin.unsafeCastToNativeObject(_buffer.nativeOwner)
-  }
-
-  /// - Precondition: The array has a native buffer.
-  @inlinable
-  @inline(__always)
-  internal func _getOwner_native() -> Builtin.NativeObject {
-#if _runtime(_ObjC)
-    if _isClassOrObjCExistential(Element.self) {
-      // We are hiding the access to '_buffer.owner' behind
-      // _getOwner() to help the compiler hoist uniqueness checks in
-      // the case of class or Objective-C existential typed array
-      // elements.
-      return _getOwnerWithSemanticLabel_native()
-    }
-#endif
-    // In the value typed case the extra call to
-    // _getOwnerWithSemanticLabel_native hinders optimization.
-    return Builtin.unsafeCastToNativeObject(_buffer.owner)
-  }
-
   @inlinable
   @_semantics("array.make_mutable")
   internal mutating func _makeMutableAndUnique() {
@@ -417,22 +371,6 @@ extension ContiguousArray {
     _buffer._checkValidSubscript(index)
   }
 
-  /// Check that the given `index` is valid for subscripting, i.e.
-  /// `0 ≤ index < count`.
-  @inlinable
-  @_semantics("array.check_subscript")
-  public // @testable
-  func _checkSubscript(
-    _ index: Int, wasNativeTypeChecked: Bool
-  ) -> _DependenceToken {
-#if _runtime(_ObjC)
-    _buffer._checkValidSubscript(index)
-#else
-    _buffer._checkValidSubscript(index)
-#endif
-    return _DependenceToken()
-  }
-
   /// Check that the specified `index` is valid, i.e. `0 ≤ index ≤ count`.
   @inlinable
   @_semantics("array.check_index")
@@ -441,21 +379,6 @@ extension ContiguousArray {
     _precondition(index >= startIndex, "Negative ContiguousArray index is out of range")
   }
 
-  @_semantics("array.get_element")
-  @inline(__always)
-  public // @testable
-  func _getElement(
-    _ index: Int,
-    wasNativeTypeChecked: Bool,
-    matchingSubscriptCheck: _DependenceToken
-  ) -> Element {
-#if false
-    return _buffer.getElement(index, wasNativeTypeChecked: wasNativeTypeChecked)
-#else
-    return _buffer.getElement(index)
-#endif
-  }
-
   @inlinable
   @_semantics("array.get_element_address")
   internal func _getElementAddress(_ index: Int) -> UnsafeMutablePointer<Element> {
diff --git a/test/Interpreter/formal_access.swift b/test/Interpreter/formal_access.swift
@@ -2,6 +2,8 @@
 // REQUIRES: executable_test
 // REQUIRES: swift_test_mode_optimize_none
 
+// REQUIRES: rdar44160503
+
 class C: CustomStringConvertible {
   var value: Int
   init(_ v: Int) { value = v }
diff --git a/test/SILGen/tuples.swift b/test/SILGen/tuples.swift
@@ -159,22 +159,19 @@ extension P {
 
 // CHECK-LABEL: sil @$S6tuples15testTupleAssign1xySaySiGz_tF : $@convention(thin) (@inout Array<Int>) -> () {
 // CHECK: [[ACCESS:%.*]] = begin_access [modify] [unknown] %0 : $*Array<Int>
-// function_ref Array.subscript.nativeOwningMutableAddressor
-// CHECK: [[ADDRESSOR:%.*]] = function_ref @$SSayxSiciao : $@convention(method) <τ_0_0> (Int, @inout Array<τ_0_0>) -> (UnsafeMutablePointer<τ_0_0>, @owned Builtin.NativeObject)
-// CHECK: [[TUPLE:%.*]] = apply [[ADDRESSOR]]<Int>(%{{.*}}, [[ACCESS]]) : $@convention(method) <τ_0_0> (Int, @inout Array<τ_0_0>) -> (UnsafeMutablePointer<τ_0_0>, @owned Builtin.NativeObject)
-// CHECK: ([[PTR:%.*]], [[OBJ:%.*]]) = destructure_tuple [[TUPLE]] : $(UnsafeMutablePointer<Int>, Builtin.NativeObject)
+// function_ref Array.subscript.modify
+// CHECK: [[ACCESSOR:%.*]] = function_ref @$SSayxSiciM : $@yield_once @convention(method) <τ_0_0> (Int, @inout Array<τ_0_0>) -> @yields @inout τ_0_0
+// CHECK: ([[VALUE:%.*]], [[TOKEN:%.*]]) = begin_apply [[ACCESSOR]]<Int>(%{{.*}}, [[ACCESS]]) : $@yield_once @convention(method) <τ_0_0> (Int, @inout Array<τ_0_0>) -> @yields @inout τ_0_0
 // CHECK: assign %{{.*}} to %{{.*}} : $*Int
+// CHECK: end_apply [[TOKEN]]
 // CHECK: end_access [[ACCESS]] : $*Array<Int>
-// CHECK: destroy_value [[OBJ]] : $Builtin.NativeObject
-//
 // CHECK: [[ACCESS:%.*]] = begin_access [modify] [unknown] %0 : $*Array<Int>
-// function_ref Array.subscript.nativeOwningMutableAddressor
-// CHECK: [[ADDRESSOR:%.*]] = function_ref @$SSayxSiciao : $@convention(method) <τ_0_0> (Int, @inout Array<τ_0_0>) -> (UnsafeMutablePointer<τ_0_0>, @owned Builtin.NativeObject)
-// CHECK: [[TUPLE:%.*]] = apply [[ADDRESSOR]]<Int>(%{{.*}}, [[ACCESS]]) : $@convention(method) <τ_0_0> (Int, @inout Array<τ_0_0>) -> (UnsafeMutablePointer<τ_0_0>, @owned Builtin.NativeObject)
-// CHECK: ([[PTR:%.*]], [[OBJ:%.*]]) = destructure_tuple [[TUPLE]] : $(UnsafeMutablePointer<Int>, Builtin.NativeObject)
+// function_ref Array.subscript.modify
+// CHECK: [[ACCESSOR:%.*]] = function_ref @$SSayxSiciM : $@yield_once @convention(method) <τ_0_0> (Int, @inout Array<τ_0_0>) -> @yields @inout τ_0_0
+// CHECK: ([[VALUE:%.*]], [[TOKEN:%.*]]) = begin_apply [[ACCESSOR]]<Int>(%{{.*}}, [[ACCESS]]) : $@yield_once @convention(method) <τ_0_0> (Int, @inout Array<τ_0_0>) -> @yields @inout τ_0_0
 // CHECK: assign %{{.*}} to %{{.*}} : $*Int
+// CHECK: end_apply [[TOKEN]]
 // CHECK: end_access [[ACCESS]] : $*Array<Int>
-// CHECK: destroy_value [[OBJ]] : $Builtin.NativeObject
 // CHECK-LABEL: } // end sil function '$S6tuples15testTupleAssign1xySaySiGz_tF'
 public func testTupleAssign(x: inout [Int]) {
   (x[0], x[1]) = (0, 1)
diff --git a/test/SILGen/writeback_conflict_diagnostics.swift b/test/SILGen/writeback_conflict_diagnostics.swift
@@ -71,12 +71,12 @@ var global_array : [[Int]]
 
 func testMultiArray(_ i : Int, j : Int, array : [[Int]]) {
   var array = array
-  swap(&array[i][j],
-       &array[i][i])
-  swap(&array[0][j],
-       &array[0][i])
-  swap(&global_array[0][j],
-       &global_array[0][i])
+  swap(&array[i][j],  // expected-note  {{concurrent writeback occurred here}}
+       &array[i][i])  // expected-error {{inout writeback through subscript occurs in multiple arguments to call, introducing invalid aliasing}}
+  swap(&array[0][j],  // expected-note  {{concurrent writeback occurred here}}
+       &array[0][i])  // expected-error {{inout writeback through subscript occurs in multiple arguments to call, introducing invalid aliasing}}
+  swap(&global_array[0][j],  // expected-note  {{concurrent writeback occurred here}}
+       &global_array[0][i])  // expected-error {{inout writeback through subscript occurs in multiple arguments to call, introducing invalid aliasing}}
   
   // TODO: This is obviously the same writeback problem, but isn't detectable
   // with the current level of sophistication in SILGen.
diff --git a/test/SILOptimizer/array_mutable_assertonly.swift b/test/SILOptimizer/array_mutable_assertonly.swift
@@ -13,6 +13,8 @@
 // RUN: %target-swift-frontend -O -emit-sil -Xllvm -debug-only=cowarray-opts -primary-file %s 2>&1 | %FileCheck %s --check-prefix=TEST13
 // REQUIRES: asserts,swift_stdlib_no_asserts,optimized_stdlib
 
+// XFAIL: linux
+
 // TEST1-LABEL: COW Array Opts in Func {{.*}}inoutarr{{.*}}
 // TEST1: Hoisting make_mutable
 // TEST1: COW Array Opts
