Skip to content

[stdlib] Change C-style for loop to Swift-style for-in loop #889

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

Closed
wants to merge 1 commit into from
Closed
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
2 changes: 1 addition & 1 deletion stdlib/public/core/CString.swift
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public func _persistCString(s: UnsafePointer<CChar>) -> [CChar]? {
}
let length = Int(_swift_stdlib_strlen(s))
var result = [CChar](count: length + 1, repeatedValue: 0)
for var i = 0; i < length; i += 1 {
for i in 0..<length {
// FIXME: this will not compile on platforms where 'CChar' is unsigned.
result[i] = s[i]
}
Expand Down
2 changes: 1 addition & 1 deletion stdlib/public/core/Character.swift
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ public struct Character :
@warn_unused_result
static func _smallSize(value: UInt64) -> Int {
var mask: UInt64 = 0xFF
for var i = 0; i < 8; i += 1 {
for i in 0..<8 {
Copy link

Choose a reason for hiding this comment

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

Is there an official style guide regarding these loops?
I find it easier to read when it's in the style of for i in 0 ..< 8 (with that spacing)

Copy link
Contributor

Choose a reason for hiding this comment

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

There’s no official guide. I will say that the standard library should remain internally consistent, I don’t see any compelling reason to change what we’re doing with range operator spacing.

Copy link

Choose a reason for hiding this comment

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

Re: internal consistency, the C-style formatting is also spaced in the way I suggested (it's not var i=0; i<8; i+=1).

Comparing 0..<3 and 0 ..< 3, I find the latter significantly easier to process in every case.
The decision ultimately doesn't affect my life in any way, but being an open forum, I wanted to put my vote in for readability.

if (value & mask) == mask {
return i
}
Expand Down
26 changes: 15 additions & 11 deletions stdlib/public/core/HashedCollections.swift.gyb
Original file line number Diff line number Diff line change
Expand Up @@ -713,7 +713,8 @@ public func == <Element : Hashable>(lhs: Set<Element>, rhs: Set<Element>) -> Boo
}

let endIndex = lhsNative.endIndex
for var i = lhsNative.startIndex; i != endIndex; i = i.successor() {
var i = lhsNative.startIndex
while i != endIndex {
let key = lhsNative.assertingGet(i)
let bridgedKey: AnyObject = _bridgeToObjectiveCUnconditional(key)
let optRhsValue: AnyObject? = rhsCocoa.maybeGet(bridgedKey)
Expand All @@ -722,6 +723,7 @@ public func == <Element : Hashable>(lhs: Set<Element>, rhs: Set<Element>) -> Boo
continue
}
}
i = i.successor()
return false
}
return true
Expand Down Expand Up @@ -1230,8 +1232,8 @@ public func == <Key : Equatable, Value : Equatable>(
}

let endIndex = lhsNative.endIndex
for var index = lhsNative.startIndex; index != endIndex;
index._successorInPlace() {
var index = lhsNative.startIndex
while index != endIndex {
let (key, value) = lhsNative.assertingGet(index)
let optRhsValue: AnyObject? =
rhsCocoa.maybeGet(_bridgeToObjectiveCUnconditional(key))
Expand All @@ -1240,6 +1242,7 @@ public func == <Key : Equatable, Value : Equatable>(
continue
}
}
index._successorInPlace()
return false
}
return true
Expand Down Expand Up @@ -2113,7 +2116,7 @@ struct _Native${Self}Storage<${TypeParametersDecl}> :
var description: String {
var result = ""
#if INTERNAL_CHECKS_ENABLED
for var i = 0; i != capacity; i += 1 {
for i in 0..<capacity {
if isInitializedEntry(i) {
let key = keyAt(i)
result += "bucket \(i), ideal bucket = \(_bucket(key)), key = \(key)\n"
Expand Down Expand Up @@ -2601,7 +2604,7 @@ final internal class _Native${Self}StorageOwner<${TypeParametersDecl}>
let bridged = _createBridgedNativeStorage(nativeStorage.capacity)

// Bridge everything.
for var i = 0; i < nativeStorage.capacity; i += 1 {
for i in 0..<nativeStorage.capacity {
if nativeStorage.isInitializedEntry(i) {
let key = _bridgeToObjectiveCUnconditional(nativeStorage.keyAt(i))
%if Self == 'Set':
Expand Down Expand Up @@ -3248,19 +3251,19 @@ internal enum _Variant${Self}Storage<${TypeParametersDecl}> : _HashStorageType {

// Find the last bucket in the contiguous chain
var lastInChain = hole
for var b = nativeStorage._next(lastInChain);
nativeStorage.isInitializedEntry(b);
b = nativeStorage._next(b) {
var b = nativeStorage._next(lastInChain)
while nativeStorage.isInitializedEntry(b) {
lastInChain = b
b = nativeStorage._next(b)
}

// Relocate out-of-place elements in the chain, repeating until
// none are found.
while hole != lastInChain {
// Walk backwards from the end of the chain looking for
// something out-of-place.
var b: Int
for b = lastInChain; b != hole; b = nativeStorage._prev(b) {
var b = lastInChain
while b != hole {
let idealBucket = nativeStorage._bucket(nativeStorage.keyAt(b))

// Does this element belong between start and hole? We need
Expand All @@ -3271,6 +3274,7 @@ internal enum _Variant${Self}Storage<${TypeParametersDecl}> : _HashStorageType {
if start <= hole ? (c0 && c1) : (c0 || c1) {
break // Found it
}
b = nativeStorage._prev(b)
}

if b == hole { // No out-of-place elements found; we're done adjusting
Expand Down Expand Up @@ -3409,7 +3413,7 @@ internal enum _Variant${Self}Storage<${TypeParametersDecl}> : _HashStorageType {
nativeStorage = native
}

for var b = 0; b != nativeStorage.capacity; b += 1 {
for b in 0..<nativeStorage.capacity {
if nativeStorage.isInitializedEntry(b) {
nativeStorage.destroyEntryAt(b)
}
Expand Down
3 changes: 2 additions & 1 deletion stdlib/public/core/StringCharacterView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ extension String.CharacterView : CollectionType {
unicodeScalars[start].value)
start._successorInPlace()

for ; start != end; start._successorInPlace() {
while start != end {
// FIXME(performance): consider removing this "fast path". A branch
// that is hard to predict could be worse for performance than a few
// loads from cache to fetch the property 'gcb1'.
Expand All @@ -158,6 +158,7 @@ extension String.CharacterView : CollectionType {
break
}
gcb0 = gcb1
start._successorInPlace()
}

return start._position - startIndexUTF16
Expand Down
6 changes: 3 additions & 3 deletions stdlib/public/core/Unicode.swift
Original file line number Diff line number Diff line change
Expand Up @@ -705,9 +705,8 @@ public func transcode<

var inputDecoder = inputEncoding.init()
var hadError = false
for var scalar = inputDecoder.decode(&input);
!scalar.isEmptyInput();
scalar = inputDecoder.decode(&input) {
var scalar = inputDecoder.decode(&input)
while !scalar.isEmptyInput() {
switch scalar {
case .Result(let us):
OutputEncoding.encode(us, output: output)
Expand All @@ -721,6 +720,7 @@ public func transcode<
hadError = true
}
}
scalar = inputDecoder.decode(&input)
}
return hadError
}
Expand Down