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.

Sign up for GitHub

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

Jump to bottom

[SR-4028] enumerateBytes gives 0 length with swift 3.1 #3886

Closed
nevil opened this issue Feb 22, 2017 · 8 comments
Closed

[SR-4028] enumerateBytes gives 0 length with swift 3.1 #3886

nevil opened this issue Feb 22, 2017 · 8 comments
Assignees
@phausler
Labels
bug Foundation

Comments

@nevil
Copy link
Contributor

nevil commented Feb 22, 2017

Previous ID SR-4028
Radar None
Original Reporter @nevil
Type Bug
Status Closed
Resolution Done
Additional Detail from JIRA
Votes 0
Component/s Foundation
Labels Bug, 3.1Regression
Assignee @phausler
Priority Medium

md5: 682108fbbdcf9923fa882f574139be25

Issue Description:

Problem:
enumerateBytes receives length value 0 when compiled with swift 3.1 from Xcode 8.3 beta 3.
With swift 3.0.1 from Xcode 8.2.1 the length is the correct length of the data buffer.

See the code at the end of the report.

Not expected behavior:
Command line tools set to "Xcode 8.3 (8W132p)" in Xcode

:> swiftc --version
Apple Swift version 3.1 (swiftlang-802.0.36.2 clang-802.0.35)
Target: x86_64-apple-macosx10.9

:> swiftc bug.swift

:> ./bug
Original length: 0
Subdata length: 0

Expected behavior:
Command line tools set to "Xcode 8.2.1 (8C1002)" in Xcode

:>  swiftc --version
Apple Swift version 3.0.2 (swiftlang-800.0.63 clang-800.0.42.1)
Target: x86_64-apple-macosx10.9

:>  swiftc bug.swift

:>  ./bug
Original length: 1000000
Subdata length: 1000

Test code, bug.swift

import Foundation

let originaldata = Data(repeating: 55, count: 1000_000)
let subdata = originaldata.subdata(in: originaldata.count - 1000 ..< originaldata.count)
originaldata.enumerateBytes { (bytes: UnsafeBufferPointer<UInt8>, length: Data.Index, stop: inout Bool) in
    print("Original length: \(length)")
}

subdata.enumerateBytes { (bytes: UnsafeBufferPointer<UInt8>, length: Data.Index, stop: inout Bool) in
    print("Subdata length: \(length)")
}
@belkadan
Copy link

cc @phausler

@belkadan
Copy link

If I'm reading the code correctly, that parameter isn't supposed to be the length; it's the offset. The fact that it used to provide the length (and still does for certain Data instances) is a bug.

@nevil
Copy link
Contributor Author

nevil commented Feb 23, 2017

@belkadan
The above code was migrated from using NSData's enumerateBytes() which gets passed a range.
As the code just worked after migration I didn't even reflect over that the type is Data.Index :-)

The documentation also doesn't really help much.

What is the purpose of the index? Should it be used to offset the UnsafeBufferPointer.baseAddress?

Thanks

@nevil
Copy link
Contributor Author

nevil commented Apr 5, 2017

@phausler
May I ask what byteIndex should be used for?

In the current code it is either hard coded to 0 or is range.length.

    public func enumerateBytes(_ block: (_ buffer: UnsafeBufferPointer<UInt8>, _ byteIndex: Data.Index, _ stop: inout Bool) -> Void) {
        var stop: Bool = false
        switch _backing {
        case .swift:
            block(UnsafeBufferPointer<UInt8>(start: _bytes?.assumingMemoryBound(to: UInt8.self), count: _length), 0, &stop)
        case .immutable:
            block(UnsafeBufferPointer<UInt8>(start: _bytes?.assumingMemoryBound(to: UInt8.self), count: _length), 0, &stop)
        case .mutable:
            block(UnsafeBufferPointer<UInt8>(start: _bytes?.assumingMemoryBound(to: UInt8.self), count: _length), 0, &stop)
        case .customReference(let d):
            d.enumerateBytes { (ptr, range, stop) in
                var stopv = false
                let bytePtr = ptr.bindMemory(to: UInt8.self, capacity: range.length)
                block(UnsafeBufferPointer(start: bytePtr, count: range.length), range.length, &stopv)
                if stopv {
                    stop.pointee = true
                }
            }
        case .customMutableReference(let d):
            d.enumerateBytes { (ptr, range, stop) in
                var stopv = false
                let bytePtr = ptr.bindMemory(to: UInt8.self, capacity: range.length)
                block(UnsafeBufferPointer(start: bytePtr, count: range.length), range.length, &stopv)
                if stopv {
                    stop.pointee = true
                }
            }
        }
    }

@phausler
Copy link
Member

phausler commented Apr 5, 2017

the byteIndex should be the start of the region. So this is a typo that custom references ought to be the range.location not the range.length for the byteIndex parameter.

Thankfully these are relatively rare in the SDK. In the interim the byte index can be calculated by summing the counts of the data while iterating (it so happens that regions are ordered)

@phausler
Copy link
Member

phausler commented Apr 5, 2017

apple/swift#8559

@nevil is there an associated radar with this?

@nevil
Copy link
Contributor Author

nevil commented Apr 6, 2017

@phausler Thanks for the response!
No I did not create a radar for this.

Should I create one?

@nevil
Copy link
Contributor Author

nevil commented May 16, 2017

Confirmed fixed in swift-4.0-DEVELOPMENT-SNAPSHOT-2017-05-11

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
@shahmishal shahmishal transferred this issue from apple/swift May 5, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@phausler phausler

Labels
bug Foundation
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@nevil @phausler @belkadan