From ee8519db66e3e5ccb4b04846b941dbf63445fa08 Mon Sep 17 00:00:00 2001 From: Tony Allevato Date: Sun, 3 Mar 2019 15:10:07 -0800 Subject: [PATCH 1/6] Implement shared string APIs. --- stdlib/public/core/CMakeLists.txt | 1 + stdlib/public/core/GroupInfo.json | 1 + stdlib/public/core/SharedString.swift | 109 ++++++++++++++++++++++ stdlib/public/core/StringStorage.swift | 36 +++++++- test/stdlib/SharedString.swift | 122 +++++++++++++++++++++++++ 5 files changed, 268 insertions(+), 1 deletion(-) create mode 100644 stdlib/public/core/SharedString.swift create mode 100644 test/stdlib/SharedString.swift diff --git a/stdlib/public/core/CMakeLists.txt b/stdlib/public/core/CMakeLists.txt index ba69e14096a69..287b08cee96d8 100644 --- a/stdlib/public/core/CMakeLists.txt +++ b/stdlib/public/core/CMakeLists.txt @@ -132,6 +132,7 @@ set(SWIFTLIB_ESSENTIAL SetStorage.swift SetVariant.swift ShadowProtocols.swift + SharedString.swift Shims.swift Slice.swift SmallBuffer.swift diff --git a/stdlib/public/core/GroupInfo.json b/stdlib/public/core/GroupInfo.json index 380cdfe83e135..101a870b3301a 100644 --- a/stdlib/public/core/GroupInfo.json +++ b/stdlib/public/core/GroupInfo.json @@ -10,6 +10,7 @@ "CharacterProperties.swift", "ICU.swift", "NormalizedCodeUnitIterator.swift", + "SharedString.swift", "SmallString.swift", "StaticString.swift", "String.swift", diff --git a/stdlib/public/core/SharedString.swift b/stdlib/public/core/SharedString.swift new file mode 100644 index 0000000000000..1442f9a009f35 --- /dev/null +++ b/stdlib/public/core/SharedString.swift @@ -0,0 +1,109 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2014 - 2019 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +extension String { + /// Calls the given closure with a `String` whose backing store is shared with + /// the UTF-8 data referenced by the given buffer pointer. + /// + /// This method does not try to repair ill-formed UTF-8 code unit sequences. + /// If any are found, the closure parameter is `nil`. + /// + /// - Parameters: + /// - buffer: An `UnsafeBufferPointer` containing the UTF-8 bytes that + /// should be shared with the created `String`. + /// - body: A closure with an optional `String` parameter that shares the + /// memory pointed to by `buffer`. The string argument is valid only for + /// the duration of the method's execution, and it may be nil if the + /// `String` could not be created (for example, if the bytes were not + /// valid UTF-8). + /// - Returns: The return value, if any, of the `body` closure parameter. + public static func withSharedUTF8( + _ buffer: UnsafeBufferPointer, + _ body: (String?) throws -> Result + ) rethrows -> Result { + return try body(String(sharingUTF8: buffer, deallocator: .none)) + } + + /// Calls the given closure with a `String` whose backing store is shared with + /// the null-terminated UTF-8 data referenced by the given pointer. + /// + /// This method does not try to repair ill-formed UTF-8 code unit sequences. + /// If any are found, the closure parameter is `nil`. + /// + /// - Parameters: + /// - cString: An `UnsafePointer` containing the null-terminated UTF-8 bytes + /// that should be shared with the created `String`. + /// - body: A closure with an optional `String` parameter that shares the + /// memory pointed to by `buffer`. The string argument is valid only for + /// the duration of the method's execution, and it may be nil if the + /// `String` could not be created (for example, if the bytes were not + /// valid UTF-8). + /// - Returns: The return value, if any, of the `body` closure parameter. + public static func withSharedNullTerminatedUTF8( + _ cString: UnsafePointer, + _ body: (String?) throws -> Result + ) rethrows -> Result { + let len = UTF8._nullCodeUnitOffset(in: cString) + let buf = UnsafeBufferPointer(start: cString, count: len) + return try withSharedUTF8(buf, body) + } +} + +extension String { + /// Creates a `String` whose backing store is shared with the UTF-8 data + /// referenced by the given buffer pointer. + /// + /// This initializer does not try to repair ill-formed UTF-8 code unit + /// sequences. If any are found, the result of the initializer is `nil`. + /// + /// - Parameters: + /// - buffer: An `UnsafeBufferPointer` containing the UTF-8 bytes that + /// should be shared with the created `String`. + /// - deallocator: The strategy used to free the buffer, or `.none`. + public init?( + sharingUTF8 buffer: UnsafeBufferPointer, + deallocator: Deallocator + ) { + guard + let baseAddress = buffer.baseAddress, + case .success(let extraInfo) = validateUTF8(buffer) + else { + return nil + } + let storage = __SharedStringStorage( + immortal: baseAddress, + countAndFlags: _StringObject.CountAndFlags( + sharedCount: buffer.count, + isASCII: extraInfo.isASCII), + deallocator: deallocator) + self.init(String(_StringGuts(storage))) + } + + /// Creates a `String` whose backing store is shared with the null-terminated + /// UTF-8 data referenced by the given buffer pointer. + /// + /// This initializer does not try to repair ill-formed UTF-8 code unit + /// sequences. If any are found, the result of the initializer is `nil`. + /// + /// - Parameters: + /// - cString: An `UnsafePointer` containing the null-terminated UTF-8 bytes + /// that should be shared with the created `String`. + /// - deallocator: The strategy used to free the buffer, or `.none`. + public init?( + sharingNullTerminatedUTF8 cString: UnsafePointer, + deallocator: Deallocator + ) { + let len = UTF8._nullCodeUnitOffset(in: cString) + let buf = UnsafeBufferPointer(start: cString, count: len) + self.init(sharingUTF8: buf, deallocator: deallocator) + } +} diff --git a/stdlib/public/core/StringStorage.swift b/stdlib/public/core/StringStorage.swift index a8480accb1160..9fc2538543afc 100644 --- a/stdlib/public/core/StringStorage.swift +++ b/stdlib/public/core/StringStorage.swift @@ -647,6 +647,33 @@ extension __StringStorage { } } +extension String { + + /// Customizes how the backing store of a shared `String` is deallocated. + public enum Deallocator { + /// No action is taken on the backing store. + case none + + /// Call `free` on the backing store. + case free + + /// A custom deallocation action is taken. + case custom((UnsafePointer, Int) -> Void) + + internal func apply(to ptr: UnsafePointer, count: Int) { + switch self { + case .none: + // Intentionally do nothing. + break + case .free: + _swift_stdlib_free(UnsafeMutableRawPointer(mutating: ptr)) + case .custom(let deleter): + deleter(ptr, count) + } + } + } +} + // For shared storage and bridging literals // NOTE: older runtimes called this class _SharedStringStorage. The two // must coexist without conflicting ObjC class names, so it was @@ -655,6 +682,7 @@ final internal class __SharedStringStorage : __SwiftNativeNSString, _AbstractStringStorage { internal var _owner: AnyObject? internal var start: UnsafePointer + internal let deallocator: String.Deallocator #if arch(i386) || arch(arm) internal var _count: Int @@ -674,7 +702,8 @@ final internal class __SharedStringStorage internal init( immortal ptr: UnsafePointer, - countAndFlags: _StringObject.CountAndFlags + countAndFlags: _StringObject.CountAndFlags, + deallocator: String.Deallocator = .none ) { self._owner = nil self.start = ptr @@ -684,10 +713,15 @@ final internal class __SharedStringStorage #else self._countAndFlags = countAndFlags #endif + self.deallocator = deallocator super.init() self._invariantCheck() } + deinit { + deallocator.apply(to: start, count: count) + } + @inline(__always) final internal var isASCII: Bool { return _countAndFlags.isASCII } diff --git a/test/stdlib/SharedString.swift b/test/stdlib/SharedString.swift new file mode 100644 index 0000000000000..4cdc8217501e8 --- /dev/null +++ b/test/stdlib/SharedString.swift @@ -0,0 +1,122 @@ +// RUN: %target-run-simple-swift +// REQUIRES: executable_test + +// +// Tests for shared string APIs +// + +import StdlibUnittest + +var SharedStringTests = TestSuite("SharedStringTests") + +SharedStringTests.test("String.withSharedUTF8") { + let ptr = UnsafeMutablePointer.allocate(capacity: 4) + defer { ptr.deallocate() } + ptr.initialize(repeating: UInt8(ascii: "a"), count: 4) + let buf = UnsafeBufferPointer(start: ptr, count: 4) + + String.withSharedUTF8(buf) { str in + expectNotNil(str) + expectEqual(str!, "aaaa") + + ptr.pointee = UInt8(ascii: "b") + expectEqual(str!, "baaa") + } +} + +SharedStringTests.test("String.withSharedUTF8 invalid UTF8") { + let ptr = UnsafeMutablePointer.allocate(capacity: 1) + defer { ptr.deallocate() } + ptr.pointee = 0x80 // orphaned continuation byte + let buf = UnsafeBufferPointer(start: ptr, count: 1) + + String.withSharedUTF8(buf) { str in + expectNil(str) + } +} + +SharedStringTests.test("String.withSharedNullTerminatedUTF8") { + let ptr = UnsafeMutablePointer.allocate(capacity: 5) + defer { ptr.deallocate() } + ptr.initialize(repeating: UInt8(ascii: "a"), count: 4) + ptr[4] = 0 + + String.withSharedNullTerminatedUTF8(ptr) { str in + expectNotNil(str) + expectEqual(str!, "aaaa") + + ptr.pointee = UInt8(ascii: "b") + expectEqual(str!, "baaa") + } +} + +SharedStringTests.test("String.withSharedNullTerminatedUTF8 invalid UTF8") { + let ptr = UnsafeMutablePointer.allocate(capacity: 2) + defer { ptr.deallocate() } + ptr[0] = 0x80 // orphaned continuation byte + ptr[1] = 0 + + String.withSharedNullTerminatedUTF8(ptr) { str in + expectNil(str) + } +} + +SharedStringTests.test("String.init(sharedUTF8:deallocator:)") { + let ptr = UnsafeMutablePointer.allocate(capacity: 4) + ptr.initialize(repeating: UInt8(ascii: "a"), count: 4) + let buf = UnsafeBufferPointer(start: ptr, count: 4) + + let str = String(sharingUTF8: buf, deallocator: .custom({ ptr, _ in + ptr.deallocate() + })) + expectNotNil(str) + expectEqual(str!, "aaaa") + + ptr.pointee = UInt8(ascii: "b") + expectEqual(str!, "baaa") + + // Verify that the deallocator was called by trying to free the memory again. + // If a crash occurs, that means the deallocator already did it. + expectCrashLater() + ptr.deallocate() +} + +SharedStringTests.test("String.withSharedUTF8 invalid UTF8") { + let ptr = UnsafeMutablePointer.allocate(capacity: 1) + defer { ptr.deallocate() } + ptr.pointee = 0x80 // orphaned continuation byte + let buf = UnsafeBufferPointer(start: ptr, count: 1) + + expectNil(String(sharingUTF8: buf, deallocator: .none)) +} + +SharedStringTests.test("String.init(sharingNullTerminatedUTF8:deallocator:)") { + let ptr = UnsafeMutablePointer.allocate(capacity: 5) + ptr.initialize(repeating: UInt8(ascii: "a"), count: 4) + ptr[4] = 0 + + let str = String(sharingNullTerminatedUTF8: ptr, deallocator: .custom({ ptr, _ in + ptr.deallocate() + })) + expectNotNil(str) + expectEqual(str!, "aaaa") + + ptr.pointee = UInt8(ascii: "b") + expectEqual(str!, "baaa") + + // Verify that the deallocator was called by trying to free the memory again. + // If a crash occurs, that means the deallocator already did it. + expectCrashLater() + ptr.deallocate() +} + +SharedStringTests.test("String.init(sharingNullTerminatedUTF8:deallocator:) invalid UTF8") { + let ptr = UnsafeMutablePointer.allocate(capacity: 2) + defer { ptr.deallocate() } + ptr[0] = 0x80 // orphaned continuation byte + ptr[1] = 0 + + expectNil(String(sharingNullTerminatedUTF8: ptr, deallocator: .none)) +} + +runAllTests() From 25fbfe0b4845dfb82d1b9667e0c9942b4ded2ea5 Mon Sep 17 00:00:00 2001 From: Tony Allevato Date: Sun, 3 Mar 2019 17:15:43 -0800 Subject: [PATCH 2/6] Remove Deallocator abstraction. --- stdlib/public/core/SharedString.swift | 69 ++++++++++++++++++++++---- stdlib/public/core/StringStorage.swift | 36 +------------- test/stdlib/SharedString.swift | 12 ++--- 3 files changed, 65 insertions(+), 52 deletions(-) diff --git a/stdlib/public/core/SharedString.swift b/stdlib/public/core/SharedString.swift index 1442f9a009f35..f36b139ca6a13 100644 --- a/stdlib/public/core/SharedString.swift +++ b/stdlib/public/core/SharedString.swift @@ -30,7 +30,7 @@ extension String { _ buffer: UnsafeBufferPointer, _ body: (String?) throws -> Result ) rethrows -> Result { - return try body(String(sharingUTF8: buffer, deallocator: .none)) + return try body(String(sharingUTF8: buffer, deallocator: { _ in })) } /// Calls the given closure with a `String` whose backing store is shared with @@ -59,8 +59,40 @@ extension String { } extension String { + internal class SharedDeallocatingOwner { + internal let pointer: UnsafePointer + internal let deallocator: (UnsafePointer) -> Void + + internal init( + pointer: UnsafePointer, + deallocator: @escaping (UnsafePointer) -> Void + ) { + self.pointer = pointer + self.deallocator = deallocator + } + + deinit { + deallocator(pointer) + } + } + + /// Creates a `String` whose backing store is shared with the UTF-8 data + /// referenced by the given buffer pointer, which will be released by calling + /// `deallocate`. + /// + /// This initializer does not try to repair ill-formed UTF-8 code unit + /// sequences. If any are found, the result of the initializer is `nil`. + /// + /// - Parameter buffer: An `UnsafeBufferPointer` containing the UTF-8 bytes + /// that should be shared with the created `String`. + public init?(sharingUTF8 buffer: UnsafeBufferPointer) { + self.init( + sharingUTF8: buffer, deallocator: { ptr in ptr.deallocate() }) + } + /// Creates a `String` whose backing store is shared with the UTF-8 data - /// referenced by the given buffer pointer. + /// referenced by the given buffer pointer, which will be released by calling + /// the `deallocator` closure. /// /// This initializer does not try to repair ill-formed UTF-8 code unit /// sequences. If any are found, the result of the initializer is `nil`. @@ -68,10 +100,11 @@ extension String { /// - Parameters: /// - buffer: An `UnsafeBufferPointer` containing the UTF-8 bytes that /// should be shared with the created `String`. - /// - deallocator: The strategy used to free the buffer, or `.none`. + /// - deallocator: A closure called to free the backing store at the end of + /// the string's lifetime. public init?( sharingUTF8 buffer: UnsafeBufferPointer, - deallocator: Deallocator + deallocator: @escaping (UnsafePointer) -> Void ) { guard let baseAddress = buffer.baseAddress, @@ -83,13 +116,30 @@ extension String { immortal: baseAddress, countAndFlags: _StringObject.CountAndFlags( sharedCount: buffer.count, - isASCII: extraInfo.isASCII), - deallocator: deallocator) + isASCII: extraInfo.isASCII)) + storage._owner = SharedDeallocatingOwner( + pointer: baseAddress, deallocator: deallocator) self.init(String(_StringGuts(storage))) } /// Creates a `String` whose backing store is shared with the null-terminated - /// UTF-8 data referenced by the given buffer pointer. + /// UTF-8 data referenced by the given buffer pointer, which will be released + /// by calling `deallocate`. + /// + /// This initializer does not try to repair ill-formed UTF-8 code unit + /// sequences. If any are found, the result of the initializer is `nil`. + /// + /// - Parameter cString: An `UnsafePointer` containing the null-terminated + /// UTF-8 bytes that should be shared with the created `String`. + public init?(sharingNullTerminatedUTF8 cString: UnsafePointer) { + self.init( + sharingNullTerminatedUTF8: cString, + deallocator: { ptr in ptr.deallocate() }) + } + + /// Creates a `String` whose backing store is shared with the null-terminated + /// UTF-8 data referenced by the given pointer, which will be released by + /// calling the `deallocator` closure. /// /// This initializer does not try to repair ill-formed UTF-8 code unit /// sequences. If any are found, the result of the initializer is `nil`. @@ -97,10 +147,11 @@ extension String { /// - Parameters: /// - cString: An `UnsafePointer` containing the null-terminated UTF-8 bytes /// that should be shared with the created `String`. - /// - deallocator: The strategy used to free the buffer, or `.none`. + /// - deallocator: A closure called to free the backing store at the end of + /// the string's lifetime. public init?( sharingNullTerminatedUTF8 cString: UnsafePointer, - deallocator: Deallocator + deallocator: @escaping (UnsafePointer) -> Void ) { let len = UTF8._nullCodeUnitOffset(in: cString) let buf = UnsafeBufferPointer(start: cString, count: len) diff --git a/stdlib/public/core/StringStorage.swift b/stdlib/public/core/StringStorage.swift index 9fc2538543afc..a8480accb1160 100644 --- a/stdlib/public/core/StringStorage.swift +++ b/stdlib/public/core/StringStorage.swift @@ -647,33 +647,6 @@ extension __StringStorage { } } -extension String { - - /// Customizes how the backing store of a shared `String` is deallocated. - public enum Deallocator { - /// No action is taken on the backing store. - case none - - /// Call `free` on the backing store. - case free - - /// A custom deallocation action is taken. - case custom((UnsafePointer, Int) -> Void) - - internal func apply(to ptr: UnsafePointer, count: Int) { - switch self { - case .none: - // Intentionally do nothing. - break - case .free: - _swift_stdlib_free(UnsafeMutableRawPointer(mutating: ptr)) - case .custom(let deleter): - deleter(ptr, count) - } - } - } -} - // For shared storage and bridging literals // NOTE: older runtimes called this class _SharedStringStorage. The two // must coexist without conflicting ObjC class names, so it was @@ -682,7 +655,6 @@ final internal class __SharedStringStorage : __SwiftNativeNSString, _AbstractStringStorage { internal var _owner: AnyObject? internal var start: UnsafePointer - internal let deallocator: String.Deallocator #if arch(i386) || arch(arm) internal var _count: Int @@ -702,8 +674,7 @@ final internal class __SharedStringStorage internal init( immortal ptr: UnsafePointer, - countAndFlags: _StringObject.CountAndFlags, - deallocator: String.Deallocator = .none + countAndFlags: _StringObject.CountAndFlags ) { self._owner = nil self.start = ptr @@ -713,15 +684,10 @@ final internal class __SharedStringStorage #else self._countAndFlags = countAndFlags #endif - self.deallocator = deallocator super.init() self._invariantCheck() } - deinit { - deallocator.apply(to: start, count: count) - } - @inline(__always) final internal var isASCII: Bool { return _countAndFlags.isASCII } diff --git a/test/stdlib/SharedString.swift b/test/stdlib/SharedString.swift index 4cdc8217501e8..16b50fa673d88 100644 --- a/test/stdlib/SharedString.swift +++ b/test/stdlib/SharedString.swift @@ -66,9 +66,7 @@ SharedStringTests.test("String.init(sharedUTF8:deallocator:)") { ptr.initialize(repeating: UInt8(ascii: "a"), count: 4) let buf = UnsafeBufferPointer(start: ptr, count: 4) - let str = String(sharingUTF8: buf, deallocator: .custom({ ptr, _ in - ptr.deallocate() - })) + let str = String(sharingUTF8: buf) expectNotNil(str) expectEqual(str!, "aaaa") @@ -87,7 +85,7 @@ SharedStringTests.test("String.withSharedUTF8 invalid UTF8") { ptr.pointee = 0x80 // orphaned continuation byte let buf = UnsafeBufferPointer(start: ptr, count: 1) - expectNil(String(sharingUTF8: buf, deallocator: .none)) + expectNil(String(sharingUTF8: buf)) } SharedStringTests.test("String.init(sharingNullTerminatedUTF8:deallocator:)") { @@ -95,9 +93,7 @@ SharedStringTests.test("String.init(sharingNullTerminatedUTF8:deallocator:)") { ptr.initialize(repeating: UInt8(ascii: "a"), count: 4) ptr[4] = 0 - let str = String(sharingNullTerminatedUTF8: ptr, deallocator: .custom({ ptr, _ in - ptr.deallocate() - })) + let str = String(sharingNullTerminatedUTF8: ptr) expectNotNil(str) expectEqual(str!, "aaaa") @@ -116,7 +112,7 @@ SharedStringTests.test("String.init(sharingNullTerminatedUTF8:deallocator:) inva ptr[0] = 0x80 // orphaned continuation byte ptr[1] = 0 - expectNil(String(sharingNullTerminatedUTF8: ptr, deallocator: .none)) + expectNil(String(sharingNullTerminatedUTF8: ptr)) } runAllTests() From f78664434a01adfe4ecd0f7d7e74425b3632be6f Mon Sep 17 00:00:00 2001 From: Tony Allevato Date: Mon, 4 Mar 2019 21:02:32 -0800 Subject: [PATCH 3/6] Replace draft APIs with init(sharingContent:owner:) --- stdlib/public/core/SharedString.swift | 136 +++----------------------- test/stdlib/SharedString.swift | 103 +++++-------------- 2 files changed, 37 insertions(+), 202 deletions(-) diff --git a/stdlib/public/core/SharedString.swift b/stdlib/public/core/SharedString.swift index f36b139ca6a13..cdd6d180b00e5 100644 --- a/stdlib/public/core/SharedString.swift +++ b/stdlib/public/core/SharedString.swift @@ -11,88 +11,14 @@ //===----------------------------------------------------------------------===// extension String { - /// Calls the given closure with a `String` whose backing store is shared with - /// the UTF-8 data referenced by the given buffer pointer. - /// - /// This method does not try to repair ill-formed UTF-8 code unit sequences. - /// If any are found, the closure parameter is `nil`. - /// - /// - Parameters: - /// - buffer: An `UnsafeBufferPointer` containing the UTF-8 bytes that - /// should be shared with the created `String`. - /// - body: A closure with an optional `String` parameter that shares the - /// memory pointed to by `buffer`. The string argument is valid only for - /// the duration of the method's execution, and it may be nil if the - /// `String` could not be created (for example, if the bytes were not - /// valid UTF-8). - /// - Returns: The return value, if any, of the `body` closure parameter. - public static func withSharedUTF8( - _ buffer: UnsafeBufferPointer, - _ body: (String?) throws -> Result - ) rethrows -> Result { - return try body(String(sharingUTF8: buffer, deallocator: { _ in })) - } - - /// Calls the given closure with a `String` whose backing store is shared with - /// the null-terminated UTF-8 data referenced by the given pointer. - /// - /// This method does not try to repair ill-formed UTF-8 code unit sequences. - /// If any are found, the closure parameter is `nil`. - /// - /// - Parameters: - /// - cString: An `UnsafePointer` containing the null-terminated UTF-8 bytes - /// that should be shared with the created `String`. - /// - body: A closure with an optional `String` parameter that shares the - /// memory pointed to by `buffer`. The string argument is valid only for - /// the duration of the method's execution, and it may be nil if the - /// `String` could not be created (for example, if the bytes were not - /// valid UTF-8). - /// - Returns: The return value, if any, of the `body` closure parameter. - public static func withSharedNullTerminatedUTF8( - _ cString: UnsafePointer, - _ body: (String?) throws -> Result - ) rethrows -> Result { - let len = UTF8._nullCodeUnitOffset(in: cString) - let buf = UnsafeBufferPointer(start: cString, count: len) - return try withSharedUTF8(buf, body) - } -} - -extension String { - internal class SharedDeallocatingOwner { - internal let pointer: UnsafePointer - internal let deallocator: (UnsafePointer) -> Void - - internal init( - pointer: UnsafePointer, - deallocator: @escaping (UnsafePointer) -> Void - ) { - self.pointer = pointer - self.deallocator = deallocator - } - - deinit { - deallocator(pointer) - } - } - /// Creates a `String` whose backing store is shared with the UTF-8 data - /// referenced by the given buffer pointer, which will be released by calling - /// `deallocate`. + /// referenced by the given buffer pointer. /// - /// This initializer does not try to repair ill-formed UTF-8 code unit - /// sequences. If any are found, the result of the initializer is `nil`. - /// - /// - Parameter buffer: An `UnsafeBufferPointer` containing the UTF-8 bytes - /// that should be shared with the created `String`. - public init?(sharingUTF8 buffer: UnsafeBufferPointer) { - self.init( - sharingUTF8: buffer, deallocator: { ptr in ptr.deallocate() }) - } - - /// Creates a `String` whose backing store is shared with the UTF-8 data - /// referenced by the given buffer pointer, which will be released by calling - /// the `deallocator` closure. + /// The `owner` argument should manage the lifetime of the shared buffer and + /// its deinitializer is responsible for deallocating `buffer`. The `String` + /// instance created by this initializer retains `owner` so that deallocation + /// occurs after the string is no longer in use. The buffer _must not_ be + /// deallocated while there are any strings sharing it. /// /// This initializer does not try to repair ill-formed UTF-8 code unit /// sequences. If any are found, the result of the initializer is `nil`. @@ -100,14 +26,13 @@ extension String { /// - Parameters: /// - buffer: An `UnsafeBufferPointer` containing the UTF-8 bytes that /// should be shared with the created `String`. - /// - deallocator: A closure called to free the backing store at the end of - /// the string's lifetime. + /// - owner: An optional object that owns the memory referenced by `buffer` + /// and is responsible for deallocating it. public init?( - sharingUTF8 buffer: UnsafeBufferPointer, - deallocator: @escaping (UnsafePointer) -> Void + sharingContent buffer: UnsafeBufferPointer, + owner: AnyObject? ) { - guard - let baseAddress = buffer.baseAddress, + guard let baseAddress = buffer.baseAddress, case .success(let extraInfo) = validateUTF8(buffer) else { return nil @@ -117,44 +42,11 @@ extension String { countAndFlags: _StringObject.CountAndFlags( sharedCount: buffer.count, isASCII: extraInfo.isASCII)) - storage._owner = SharedDeallocatingOwner( - pointer: baseAddress, deallocator: deallocator) + storage._owner = owner self.init(String(_StringGuts(storage))) } +} - /// Creates a `String` whose backing store is shared with the null-terminated - /// UTF-8 data referenced by the given buffer pointer, which will be released - /// by calling `deallocate`. - /// - /// This initializer does not try to repair ill-formed UTF-8 code unit - /// sequences. If any are found, the result of the initializer is `nil`. - /// - /// - Parameter cString: An `UnsafePointer` containing the null-terminated - /// UTF-8 bytes that should be shared with the created `String`. - public init?(sharingNullTerminatedUTF8 cString: UnsafePointer) { - self.init( - sharingNullTerminatedUTF8: cString, - deallocator: { ptr in ptr.deallocate() }) - } +extension Substring { - /// Creates a `String` whose backing store is shared with the null-terminated - /// UTF-8 data referenced by the given pointer, which will be released by - /// calling the `deallocator` closure. - /// - /// This initializer does not try to repair ill-formed UTF-8 code unit - /// sequences. If any are found, the result of the initializer is `nil`. - /// - /// - Parameters: - /// - cString: An `UnsafePointer` containing the null-terminated UTF-8 bytes - /// that should be shared with the created `String`. - /// - deallocator: A closure called to free the backing store at the end of - /// the string's lifetime. - public init?( - sharingNullTerminatedUTF8 cString: UnsafePointer, - deallocator: @escaping (UnsafePointer) -> Void - ) { - let len = UTF8._nullCodeUnitOffset(in: cString) - let buf = UnsafeBufferPointer(start: cString, count: len) - self.init(sharingUTF8: buf, deallocator: deallocator) - } } diff --git a/test/stdlib/SharedString.swift b/test/stdlib/SharedString.swift index 16b50fa673d88..8d0bf54a6d697 100644 --- a/test/stdlib/SharedString.swift +++ b/test/stdlib/SharedString.swift @@ -9,110 +9,53 @@ import StdlibUnittest var SharedStringTests = TestSuite("SharedStringTests") -SharedStringTests.test("String.withSharedUTF8") { +func makeValidUTF8Buffer() -> UnsafeBufferPointer { let ptr = UnsafeMutablePointer.allocate(capacity: 4) - defer { ptr.deallocate() } ptr.initialize(repeating: UInt8(ascii: "a"), count: 4) - let buf = UnsafeBufferPointer(start: ptr, count: 4) - - String.withSharedUTF8(buf) { str in - expectNotNil(str) - expectEqual(str!, "aaaa") - - ptr.pointee = UInt8(ascii: "b") - expectEqual(str!, "baaa") - } + return UnsafeBufferPointer(start: ptr, count: 4) } -SharedStringTests.test("String.withSharedUTF8 invalid UTF8") { +func makeInvalidUTF8Buffer() -> UnsafeBufferPointer { let ptr = UnsafeMutablePointer.allocate(capacity: 1) - defer { ptr.deallocate() } ptr.pointee = 0x80 // orphaned continuation byte - let buf = UnsafeBufferPointer(start: ptr, count: 1) - - String.withSharedUTF8(buf) { str in - expectNil(str) - } + return UnsafeBufferPointer(start: ptr, count: 1) } -SharedStringTests.test("String.withSharedNullTerminatedUTF8") { - let ptr = UnsafeMutablePointer.allocate(capacity: 5) - defer { ptr.deallocate() } - ptr.initialize(repeating: UInt8(ascii: "a"), count: 4) - ptr[4] = 0 - - String.withSharedNullTerminatedUTF8(ptr) { str in - expectNotNil(str) - expectEqual(str!, "aaaa") +class BufferDeallocator { + let buffer: UnsafeBufferPointer - ptr.pointee = UInt8(ascii: "b") - expectEqual(str!, "baaa") + init(_ buffer: UnsafeBufferPointer) { + self.buffer = buffer } -} - -SharedStringTests.test("String.withSharedNullTerminatedUTF8 invalid UTF8") { - let ptr = UnsafeMutablePointer.allocate(capacity: 2) - defer { ptr.deallocate() } - ptr[0] = 0x80 // orphaned continuation byte - ptr[1] = 0 - String.withSharedNullTerminatedUTF8(ptr) { str in - expectNil(str) + deinit { + buffer.deallocate() } } -SharedStringTests.test("String.init(sharedUTF8:deallocator:)") { - let ptr = UnsafeMutablePointer.allocate(capacity: 4) - ptr.initialize(repeating: UInt8(ascii: "a"), count: 4) - let buf = UnsafeBufferPointer(start: ptr, count: 4) +SharedStringTests.test("String.init(sharing:owner:)") { + let buf = makeValidUTF8Buffer() + let str = String(sharingContent: buf, owner: BufferDeallocator(buf)) - let str = String(sharingUTF8: buf) expectNotNil(str) expectEqual(str!, "aaaa") - ptr.pointee = UInt8(ascii: "b") + // Show that the string didn't copy the buffer by modifying it in-place. + UnsafeMutableBufferPointer(mutating: buf)[0] = UInt8(ascii: "b") expectEqual(str!, "baaa") - // Verify that the deallocator was called by trying to free the memory again. - // If a crash occurs, that means the deallocator already did it. - expectCrashLater() - ptr.deallocate() -} - -SharedStringTests.test("String.withSharedUTF8 invalid UTF8") { - let ptr = UnsafeMutablePointer.allocate(capacity: 1) - defer { ptr.deallocate() } - ptr.pointee = 0x80 // orphaned continuation byte - let buf = UnsafeBufferPointer(start: ptr, count: 1) - - expectNil(String(sharingUTF8: buf)) -} - -SharedStringTests.test("String.init(sharingNullTerminatedUTF8:deallocator:)") { - let ptr = UnsafeMutablePointer.allocate(capacity: 5) - ptr.initialize(repeating: UInt8(ascii: "a"), count: 4) - ptr[4] = 0 - - let str = String(sharingNullTerminatedUTF8: ptr) - expectNotNil(str) - expectEqual(str!, "aaaa") - - ptr.pointee = UInt8(ascii: "b") + // Show that mutating a copy works as expected. + var copy = str! + copy.append("c") + expectEqual(copy, "baaac") expectEqual(str!, "baaa") - - // Verify that the deallocator was called by trying to free the memory again. - // If a crash occurs, that means the deallocator already did it. - expectCrashLater() - ptr.deallocate() } -SharedStringTests.test("String.init(sharingNullTerminatedUTF8:deallocator:) invalid UTF8") { - let ptr = UnsafeMutablePointer.allocate(capacity: 2) - defer { ptr.deallocate() } - ptr[0] = 0x80 // orphaned continuation byte - ptr[1] = 0 +SharedStringTests.test("String.init(sharing:owner:) invalid UTF8") { + let buf = makeInvalidUTF8Buffer() + let str = String(sharingContent: buf, owner: BufferDeallocator(buf)) - expectNil(String(sharingNullTerminatedUTF8: ptr)) + expectNil(str) } runAllTests() From f6922be99b272f216e83292b93ee381c2350e891 Mon Sep 17 00:00:00 2001 From: Tony Allevato Date: Tue, 5 Mar 2019 11:26:05 -0800 Subject: [PATCH 4/6] Add partial Substring.withSharedString impl. --- stdlib/public/core/SharedString.swift | 30 +++++++++++++++++++++++++++ test/stdlib/SharedString.swift | 9 ++++++++ 2 files changed, 39 insertions(+) diff --git a/stdlib/public/core/SharedString.swift b/stdlib/public/core/SharedString.swift index cdd6d180b00e5..c0c32ab0a72ac 100644 --- a/stdlib/public/core/SharedString.swift +++ b/stdlib/public/core/SharedString.swift @@ -48,5 +48,35 @@ extension String { } extension Substring { + /// Calls the given closure, passing it a `String` whose contents are equal to + /// the substring but which shares the substring's storage instead of copying + /// it. + /// + /// The `String` value passed into the closure is only valid durings its + /// execution. + /// + /// - Parameter body: A closure with a `String` parameter that shares the + /// storage of the substring. If `body` has a return value, that value is + /// also used as the return value for the `withSharedString(_:)` method. The + /// argument is valid only for the duration of the closure's execution. + /// - Returns: The return value, if any, of the `body` closure parameter. + public func withSharedString( + _ body: (String) throws -> Result + ) rethrows -> Result { + guard _wholeGuts.isFastUTF8 else { + // TODO: What's the right way to get the pointer for a non-fast-UTF-8 + // string here? + return try body(String(self)) + } + return try _wholeGuts.withFastUTF8 { utf8 in + let storage = __SharedStringStorage( + immortal: utf8.baseAddress! + self._offsetRange.lowerBound, + countAndFlags: _StringObject.CountAndFlags( + sharedCount: self._offsetRange.count, + isASCII: _wholeGuts.isASCII)) + let str = String(_StringGuts(storage)) + return try body(str) + } + } } diff --git a/test/stdlib/SharedString.swift b/test/stdlib/SharedString.swift index 8d0bf54a6d697..20aacf6e29454 100644 --- a/test/stdlib/SharedString.swift +++ b/test/stdlib/SharedString.swift @@ -58,4 +58,13 @@ SharedStringTests.test("String.init(sharing:owner:) invalid UTF8") { expectNil(str) } +SharedStringTests.test("Substring.withSharedString(_:)") { + let original = "abcde" + let substr = original.dropFirst().dropLast() + + substr.withSharedString { shared in + expectEqual(shared, "bcd") + } +} + runAllTests() From 5b9d088b130af37a6bccaabcdc8f36334058e801 Mon Sep 17 00:00:00 2001 From: Tony Allevato Date: Tue, 5 Mar 2019 13:49:07 -0800 Subject: [PATCH 5/6] Add String.init(sharing:) with [UInt8] argument. --- stdlib/public/core/SharedString.swift | 22 ++++++++++++++++++++++ test/stdlib/SharedString.swift | 24 ++++++++++++++++++++++-- 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/stdlib/public/core/SharedString.swift b/stdlib/public/core/SharedString.swift index c0c32ab0a72ac..93a06d196b06c 100644 --- a/stdlib/public/core/SharedString.swift +++ b/stdlib/public/core/SharedString.swift @@ -45,6 +45,28 @@ extension String { storage._owner = owner self.init(String(_StringGuts(storage))) } + + /// Creates a `String` whose backing store is shared with the UTF-8 data in + /// the given array. + /// + /// The new `String` instance shares ownership of the array's underlying + /// storage, guaranteeing that the `String` remains valid even if the `array` + /// is released. + /// + /// This initializer does not try to repair ill-formed UTF-8 code unit + /// sequences. If any are found, the result of the initializer is `nil`. + /// + /// - Parameter array: An `Array` containing the UTF-8 bytes that should be + /// shared with the created `String`. + public init?(sharing array: [UInt8]) { + guard case (let owner, let rawPtr?) = array._cPointerArgs() else { + self = "" + return + } + let baseAddress = rawPtr.assumingMemoryBound(to: UInt8.self) + let buffer = UnsafeBufferPointer(start: baseAddress, count: array.count) + self.init(sharingContent: buffer, owner: owner) + } } extension Substring { diff --git a/test/stdlib/SharedString.swift b/test/stdlib/SharedString.swift index 20aacf6e29454..b40619fe613fd 100644 --- a/test/stdlib/SharedString.swift +++ b/test/stdlib/SharedString.swift @@ -33,7 +33,7 @@ class BufferDeallocator { } } -SharedStringTests.test("String.init(sharing:owner:)") { +SharedStringTests.test("String.init(sharingContent:owner:)") { let buf = makeValidUTF8Buffer() let str = String(sharingContent: buf, owner: BufferDeallocator(buf)) @@ -51,7 +51,7 @@ SharedStringTests.test("String.init(sharing:owner:)") { expectEqual(str!, "baaa") } -SharedStringTests.test("String.init(sharing:owner:) invalid UTF8") { +SharedStringTests.test("String.init(sharingContent:owner:) invalid UTF8") { let buf = makeInvalidUTF8Buffer() let str = String(sharingContent: buf, owner: BufferDeallocator(buf)) @@ -67,4 +67,24 @@ SharedStringTests.test("Substring.withSharedString(_:)") { } } +SharedStringTests.test("String.init(sharing:)") { + var array = [UInt8](repeating: UInt8(ascii: "a"), count: 4) + let str = String(sharing: array) + + expectNotNil(str) + expectEqual(str!, "aaaa") + + // Show that mutating the array causes CoW and the original string isn't + // modified. + array[0] = UInt8(ascii: "b") + expectEqual(str!, "aaaa") +} + +SharedStringTests.test("String.init(sharing:) invalid UTF8") { + let array: [UInt8] = [0x80] // orphaned continuation byte + let str = String(sharing: array) + + expectNil(str) +} + runAllTests() From dc2335ed5dfeeb30731bcb32a9b82ecc5f1bff81 Mon Sep 17 00:00:00 2001 From: Tony Allevato Date: Tue, 5 Mar 2019 13:50:10 -0800 Subject: [PATCH 6/6] Expected values come first in StdlibUnittest. --- test/stdlib/SharedString.swift | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/stdlib/SharedString.swift b/test/stdlib/SharedString.swift index b40619fe613fd..560effa4feeb9 100644 --- a/test/stdlib/SharedString.swift +++ b/test/stdlib/SharedString.swift @@ -38,17 +38,17 @@ SharedStringTests.test("String.init(sharingContent:owner:)") { let str = String(sharingContent: buf, owner: BufferDeallocator(buf)) expectNotNil(str) - expectEqual(str!, "aaaa") + expectEqual("aaaa", str!) // Show that the string didn't copy the buffer by modifying it in-place. UnsafeMutableBufferPointer(mutating: buf)[0] = UInt8(ascii: "b") - expectEqual(str!, "baaa") + expectEqual("baaa", str!) // Show that mutating a copy works as expected. var copy = str! copy.append("c") - expectEqual(copy, "baaac") - expectEqual(str!, "baaa") + expectEqual("baaac", copy) + expectEqual("baaa", str!) } SharedStringTests.test("String.init(sharingContent:owner:) invalid UTF8") { @@ -63,7 +63,7 @@ SharedStringTests.test("Substring.withSharedString(_:)") { let substr = original.dropFirst().dropLast() substr.withSharedString { shared in - expectEqual(shared, "bcd") + expectEqual("bcd", shared) } } @@ -72,12 +72,12 @@ SharedStringTests.test("String.init(sharing:)") { let str = String(sharing: array) expectNotNil(str) - expectEqual(str!, "aaaa") + expectEqual("aaaa", str!) // Show that mutating the array causes CoW and the original string isn't // modified. array[0] = UInt8(ascii: "b") - expectEqual(str!, "aaaa") + expectEqual("aaaa", str!) } SharedStringTests.test("String.init(sharing:) invalid UTF8") {