Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SE-0134][stdlib] Rename/remove two properties on String #3816

Merged
merged 4 commits into from
Jul 28, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions stdlib/private/SwiftPrivate/IO.swift
Original file line number Diff line number Diff line change
Expand Up @@ -100,14 +100,14 @@ public struct _FDOutputStream : TextOutputStream {
}

public mutating func write(_ string: String) {
let utf8 = string.nulTerminatedUTF8
utf8.withUnsafeBufferPointer {
(utf8) -> Void in
let utf8CStr = string.utf8CString
utf8CStr.withUnsafeBufferPointer {
(utf8CStr) -> Void in
var writtenBytes = 0
let bufferSize = utf8.count - 1
let bufferSize = utf8CStr.count - 1
while writtenBytes != bufferSize {
let result = _swift_stdlib_write(
self.fd, UnsafePointer(utf8.baseAddress! + Int(writtenBytes)),
self.fd, UnsafeRawPointer(utf8CStr.baseAddress! + Int(writtenBytes)),
bufferSize - writtenBytes)
if result < 0 {
fatalError("write() returned an error")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ public func _stdlib_mkstemps(_ template: inout String, _ suffixlen: CInt) -> CIn
#if os(Android)
preconditionFailure("mkstemps doesn't work on Android")
#else
var utf8 = template.nulTerminatedUTF8
let (fd, fileName) = utf8.withUnsafeMutableBufferPointer {
(utf8) -> (CInt, String) in
let fd = mkstemps(UnsafeMutablePointer(utf8.baseAddress!), suffixlen)
let fileName = String(cString: utf8.baseAddress!)
var utf8CStr = template.utf8CString
let (fd, fileName) = utf8CStr.withUnsafeMutableBufferPointer {
(utf8CStr) -> (CInt, String) in
let fd = mkstemps(utf8CStr.baseAddress!, suffixlen)
let fileName = String(cString: utf8CStr.baseAddress!)
return (fd, fileName)
}
template = fileName
Expand Down
4 changes: 2 additions & 2 deletions stdlib/public/core/LifetimeManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ extension String {
public func withCString<Result>(
_ body: @noescape (UnsafePointer<Int8>) throws -> Result
) rethrows -> Result {
return try self.nulTerminatedUTF8.withUnsafeBufferPointer {
try body(UnsafePointer($0.baseAddress!))
return try self.utf8CString.withUnsafeBufferPointer {
try body($0.baseAddress!)
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion stdlib/public/core/Pointer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,6 @@ public // COMPILER_INTRINSIC
func _convertConstStringToUTF8PointerArgument<
ToPointer : _Pointer
>(_ str: String) -> (AnyObject?, ToPointer) {
let utf8 = Array(str.nulTerminatedUTF8)
let utf8 = Array(str.utf8CString)
return _convertConstArrayToPointerArgument(utf8)
}
40 changes: 18 additions & 22 deletions stdlib/public/core/StringUTF8.swift
Original file line number Diff line number Diff line change
Expand Up @@ -383,35 +383,21 @@ extension String {
return _core.elementWidth == 1 ? _core.startASCII : nil
}

/// A contiguously stored null-terminated UTF-8 representation of
/// the string.
/// A contiguously stored null-terminated UTF-8 representation of the string.
///
/// To access the underlying memory, invoke
/// `withUnsafeBufferPointer` on the array.
/// To access the underlying memory, invoke `withUnsafeBufferPointer` on the
/// array.
///
/// let s = "Hello!"
/// let bytes = s.nulTerminatedUTF8
/// let bytes = s.utf8CString
/// print(bytes)
/// // Prints "[72, 101, 108, 108, 111, 33, 0]"
///
///
/// bytes.withUnsafeBufferPointer { ptr in
/// print(strlen(UnsafePointer(ptr.baseAddress!)))
/// print(strlen(ptr.baseAddress!))
/// }
/// // Prints "6"
public var nulTerminatedUTF8: ContiguousArray<UTF8.CodeUnit> {
var result = ContiguousArray<UTF8.CodeUnit>()
result.reserveCapacity(utf8.count + 1)
result += utf8
result.append(0)
return result
}

/// A contiguously stored null-terminated UTF-8 representation of
/// the string.
///
/// This is a variation on nulTerminatedUTF8 that creates an array
/// of element type CChar for use with CString API's.
public var nulTerminatedUTF8CString: ContiguousArray<CChar> {
public var utf8CString: ContiguousArray<CChar> {
var result = ContiguousArray<CChar>()
result.reserveCapacity(utf8.count + 1)
for c in utf8 {
Expand All @@ -428,7 +414,11 @@ extension String {
if ptr != nil {
return try body(UnsafeBufferPointer(start: ptr, count: _core.count))
}
return try nulTerminatedUTF8.withUnsafeBufferPointer(body)
var nullTerminatedUTF8 = ContiguousArray<UTF8.CodeUnit>()
nullTerminatedUTF8.reserveCapacity(utf8.count + 1)
nullTerminatedUTF8 += utf8
nullTerminatedUTF8.append(0)
return try nullTerminatedUTF8.withUnsafeBufferPointer(body)
}

/// Creates a string corresponding to the given sequence of UTF-8 code units.
Expand Down Expand Up @@ -725,3 +715,9 @@ extension String.UTF8View : CustomPlaygroundQuickLookable {
}
}

extension String {
@available(*, unavailable, message: "Please use String.utf8CString instead.")
public var nulTerminatedUTF8: ContiguousArray<UTF8.CodeUnit> {
Builtin.unreachable()
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need to mark nulTerminatedUTF8CString unavailable, just delete it. It wasn't on tree long enough for anyone to use it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done and done.

12 changes: 6 additions & 6 deletions test/1_stdlib/Runtime.swift.gyb
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,20 @@ import SwiftShims
@_silgen_name("swift_demangle")
public
func _stdlib_demangleImpl(
mangledName: UnsafePointer<UInt8>?,
mangledName: UnsafePointer<CChar>?,
mangledNameLength: UInt,
outputBuffer: UnsafeMutablePointer<UInt8>?,
outputBuffer: UnsafeMutablePointer<CChar>?,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of my depth with @_silgen_name; is this OK?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, your version of the code is more correct than what was there before...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is good.

outputBufferSize: UnsafeMutablePointer<UInt>?,
flags: UInt32
) -> UnsafeMutablePointer<CChar>?

func _stdlib_demangleName(_ mangledName: String) -> String {
return mangledName.nulTerminatedUTF8.withUnsafeBufferPointer {
(mangledNameUTF8) in
return mangledName.utf8CString.withUnsafeBufferPointer {
(mangledNameUTF8CStr) in

let demangledNamePtr = _stdlib_demangleImpl(
mangledName: mangledNameUTF8.baseAddress,
mangledNameLength: UInt(mangledNameUTF8.count - 1),
mangledName: mangledNameUTF8CStr.baseAddress,
mangledNameLength: UInt(mangledNameUTF8CStr.count - 1),
outputBuffer: nil,
outputBufferSize: nil,
flags: 0)
Expand Down
8 changes: 3 additions & 5 deletions test/1_stdlib/StringAPI.swift
Original file line number Diff line number Diff line change
Expand Up @@ -483,19 +483,17 @@ CStringTests.test("String.decodeCString") {
}
}

CStringTests.test("String.nulTerminatedUTF8") {
CStringTests.test("String.utf8CString") {
do {
let (cstr, dealloc) = getASCIICString()
let str = String(cString: cstr)
expectEqualCString(cstr, str.nulTerminatedUTF8)
expectEqualCString(cstr, str.nulTerminatedUTF8CString)
expectEqualCString(cstr, str.utf8CString)
dealloc()
}
do {
let (cstr, dealloc) = getNonASCIICString()
let str = String(cString: cstr)
expectEqualCString(cstr, str.nulTerminatedUTF8)
expectEqualCString(cstr, str.nulTerminatedUTF8CString)
expectEqualCString(cstr, str.utf8CString)
dealloc()
}
}
Expand Down
8 changes: 5 additions & 3 deletions test/1_stdlib/TestData.swift
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,11 @@ class TestData : TestDataSuper {
// String of course has its own way to get data, but this way tests our own data struct
func dataFrom(_ string : String) -> Data {
// Create a Data out of those bytes
return string.nulTerminatedUTF8.withUnsafeBufferPointer { (ptr) in
// Subtract 1 so we don't get the null terminator byte. This matches NSString behavior.
return Data(bytes: ptr.baseAddress!, count: ptr.count - 1)
return string.utf8CString.withUnsafeBufferPointer { (ptr) in
ptr.baseAddress!.withMemoryRebound(to: UInt8.self, capacity: ptr.count) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Annoyingly, Data(bytes: UnsafeRawPointer, count: Int) isn't available in this repo as it now is in swift-corelibs-foundation. Should I address that in this PR too?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that issue is separate.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@parkera What is the meaning of this? How can corelibs-foundation and Foundation overlay vend different APIs? I would love to change the Foundation overlay to "bytes: UnsafeRawPointer" so that it's usable without breaking the memory model. That change would be mostly source compatible, but, strictly speaking it is source breaking, thus verboten.

// Subtract 1 so we don't get the null terminator byte. This matches NSString behavior.
return Data(bytes: $0, count: ptr.count - 1)
}
}
}

Expand Down