From 51c2bd0a1ce175e7a1f16e0c2003f758110c1c62 Mon Sep 17 00:00:00 2001 From: Erik Eckstein Date: Mon, 9 Nov 2020 15:29:41 +0100 Subject: [PATCH 1/2] stdlib: remove a wrong internal check for COW array mutation Don't complain if the empty array buffer singleton is set to mutable. This can happen if reserveCapacity is called with a 0-capacity for an empty array. In this case the resulting array is still the empty array singleton. For the compiler, it's safe to treat the empty array singleton as "mutable", because there is never anything written to an array with zero capacity: all mutating array operations do any form of capacity/size > 0 check in addition to the uniqueness check. Unfortunately the empty array singleton sometimes needs such special handling with is not obvious (not to say hacky). This wrong check only fired in very specific optimization scenarios, where the inliner and the COW optimizations must play together in a certain way. I could not find an isolated test case for this problem. rdar://problem/71107908 --- stdlib/public/core/ContiguousArrayBuffer.swift | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/stdlib/public/core/ContiguousArrayBuffer.swift b/stdlib/public/core/ContiguousArrayBuffer.swift index 88a35ac497f44..b8bbb5c541d4c 100644 --- a/stdlib/public/core/ContiguousArrayBuffer.swift +++ b/stdlib/public/core/ContiguousArrayBuffer.swift @@ -470,18 +470,17 @@ internal struct _ContiguousArrayBuffer: _ArrayBufferProtocol { nonmutating set { if #available(macOS 9999, iOS 9999, watchOS 9999, tvOS 9999, *) { if (_COWChecksEnabled()) { - if newValue { - if capacity > 0 { - let wasImmutable = _swift_setImmutableCOWBuffer(_storage, true) + // Make sure to not modify the empty array singleton (which has a + // capacity of 0). + if capacity > 0 { + let wasImmutable = _swift_setImmutableCOWBuffer(_storage, newValue) + if newValue { _internalInvariant(!wasImmutable, "re-setting immutable array buffer to immutable") + } else { + _internalInvariant(wasImmutable, + "re-setting mutable array buffer to mutable") } - } else { - _internalInvariant(capacity > 0, - "setting empty array buffer to mutable") - let wasImmutable = _swift_setImmutableCOWBuffer(_storage, false) - _internalInvariant(wasImmutable, - "re-setting mutable array buffer to mutable") } } } From 6d0bbba1567dbbb6003dbf20a83ed5e8036a9bf9 Mon Sep 17 00:00:00 2001 From: Erik Eckstein Date: Mon, 9 Nov 2020 15:30:23 +0100 Subject: [PATCH 2/2] stdlib: update comments for the array buffer endCowMutation functions. --- stdlib/public/core/ArrayBuffer.swift | 2 +- stdlib/public/core/ContiguousArrayBuffer.swift | 2 +- stdlib/public/core/SliceBuffer.swift | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/stdlib/public/core/ArrayBuffer.swift b/stdlib/public/core/ArrayBuffer.swift index 29fd636ebc1d7..074d8e77980db 100644 --- a/stdlib/public/core/ArrayBuffer.swift +++ b/stdlib/public/core/ArrayBuffer.swift @@ -138,7 +138,7 @@ extension _ArrayBuffer { /// Puts the buffer in an immutable state. /// - /// - Precondition: The buffer must be mutable. + /// - Precondition: The buffer must be mutable or the empty array singleton. /// /// - Warning: After a call to `endCOWMutation` the buffer must not be mutated /// until the next call of `beginCOWMutation`. diff --git a/stdlib/public/core/ContiguousArrayBuffer.swift b/stdlib/public/core/ContiguousArrayBuffer.swift index b8bbb5c541d4c..90df5c561116d 100644 --- a/stdlib/public/core/ContiguousArrayBuffer.swift +++ b/stdlib/public/core/ContiguousArrayBuffer.swift @@ -697,7 +697,7 @@ internal struct _ContiguousArrayBuffer: _ArrayBufferProtocol { /// Puts the buffer in an immutable state. /// - /// - Precondition: The buffer must be mutable. + /// - Precondition: The buffer must be mutable or the empty array singleton. /// /// - Warning: After a call to `endCOWMutation` the buffer must not be mutated /// until the next call of `beginCOWMutation`. diff --git a/stdlib/public/core/SliceBuffer.swift b/stdlib/public/core/SliceBuffer.swift index fd19d31646218..6da4a339bb7e4 100644 --- a/stdlib/public/core/SliceBuffer.swift +++ b/stdlib/public/core/SliceBuffer.swift @@ -329,7 +329,7 @@ internal struct _SliceBuffer /// Puts the buffer in an immutable state. /// - /// - Precondition: The buffer must be mutable. + /// - Precondition: The buffer must be mutable or the empty array singleton. /// /// - Warning: After a call to `endCOWMutation` the buffer must not be mutated /// until the next call of `beginCOWMutation`.