Skip to content
41 changes: 19 additions & 22 deletions stdlib/public/core/ArrayBuffer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -247,29 +247,26 @@ extension _ArrayBuffer {
// Look for contiguous storage in the NSArray
let nonNative = self._nonNative
let cocoa = _CocoaArrayWrapper(nonNative)
let cocoaStorageBaseAddress = cocoa.contiguousStorage(self.indices)

if cocoaStorageBaseAddress != nil {
return _SliceBuffer(
owner: nonNative,
subscriptBaseAddress: UnsafeMutablePointer(cocoaStorageBaseAddress),
indices: subRange,
hasNativeBuffer: false)
guard let cocoaStorageBaseAddress = cocoa.contiguousStorage(self.indices) else {
Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer this the old way. guard is supposed to be a quick-exit path, and in this case, if plays that role well.

// No contiguous storage found; we must allocate
let subRangeCount = subRange.count
let result = _ContiguousArrayBuffer<Element>(
count: subRangeCount, minimumCapacity: 0)

// Tell Cocoa to copy the objects into our storage
cocoa.buffer.getObjects(
UnsafeMutablePointer(result.firstElementAddress),
range: _SwiftNSRange(
location: subRange.startIndex,
length: subRangeCount))

return _SliceBuffer(result, shiftedToStartIndex: subRange.startIndex)
}

// No contiguous storage found; we must allocate
let subRangeCount = subRange.count
let result = _ContiguousArrayBuffer<Element>(
count: subRangeCount, minimumCapacity: 0)

// Tell Cocoa to copy the objects into our storage
cocoa.buffer.getObjects(
UnsafeMutablePointer(result.firstElementAddress),
range: _SwiftNSRange(
location: subRange.startIndex,
length: subRangeCount))

return _SliceBuffer(result, shiftedToStartIndex: subRange.startIndex)
return _SliceBuffer(
owner: nonNative,
subscriptBaseAddress: UnsafeMutablePointer(cocoaStorageBaseAddress),
indices: subRange,
hasNativeBuffer: false)
}
set {
fatalError("not implemented")
Expand Down
12 changes: 5 additions & 7 deletions stdlib/public/core/Collection.swift
Original file line number Diff line number Diff line change
Expand Up @@ -584,19 +584,17 @@ extension SequenceType
// A fast implementation for when you are backed by a contiguous array.
public func _initializeTo(ptr: UnsafeMutablePointer<Generator.Element>)
-> UnsafeMutablePointer<Generator.Element> {
let s = self._baseAddressIfContiguous
if s != nil {
let count = self.count
ptr.initializeFrom(s, count: count)
_fixLifetime(self._owner)
return ptr + count
} else {
guard let s = self._baseAddressIfContiguous else {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't feel this is an improvement either, because of the non-trivial amount of code in the guard-else block.

var p = ptr
for x in self {
p++.initialize(x)
}
return p
}
let count = self.count
ptr.initializeFrom(s, count: count)
_fixLifetime(self._owner)
return ptr + count
}
}

Expand Down
7 changes: 3 additions & 4 deletions stdlib/public/core/Map.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,10 @@ public struct LazyMapGenerator<
/// since the copy was made, and no preceding call to `self.next()`
/// has returned `nil`.
public mutating func next() -> Element? {
let x = _base.next()
if x != nil {
return _transform(x!)
guard let x = _base.next() else {
return nil
}
return nil
return _transform(x)
Copy link
Contributor

Choose a reason for hiding this comment

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

This part LGTM.

}
Copy link
Contributor

Choose a reason for hiding this comment

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

IMHO a cleaner form for this simple expression (one statement for each branch) would be:

if let x = _base.next() {
    return _transform(x)
} else {
    return nil
}

This clarifies which branch does which.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1, I like this even better.

Choose a reason for hiding this comment

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

In your example you honestly don't even need the else since return in the if statement is already exiting the current scope.

if let x = _base.next() {
    return _transform(x)
}
return nil

Copy link
Contributor

Choose a reason for hiding this comment

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

This was improved in #193 already.


public var base: Base { return _base }
Expand Down
9 changes: 4 additions & 5 deletions stdlib/public/core/Runtime.swift.gyb
Original file line number Diff line number Diff line change
Expand Up @@ -247,12 +247,11 @@ public // @testable
func _stdlib_atomicLoadARCRef(
object target: UnsafeMutablePointer<AnyObject?>
) -> AnyObject? {
let result = _swift_stdlib_atomicLoadPtrImpl(
object: UnsafeMutablePointer(target))
if result != nil {
return Unmanaged<AnyObject>.fromOpaque(result).takeUnretainedValue()
guard let result = _swift_stdlib_atomicLoadPtrImpl(
object: UnsafeMutablePointer(target)) {
return nil
}
return nil
return Unmanaged<AnyObject>.fromOpaque(result).takeUnretainedValue()
Copy link
Contributor

Choose a reason for hiding this comment

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

This part LGTM.

}

% for operation in [ 'Add', 'And', 'Or', 'Xor' ]:
Expand Down