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

[SR-8883] ArraySlice/Data subscript behavior is not intuitive and dangerous #51389

Open
swift-ci opened this issue Sep 29, 2018 · 2 comments
Open
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. standard library Area: Standard library umbrella

Comments

@swift-ci
Copy link
Collaborator

Previous ID SR-8883
Radar None
Original Reporter eaigner (JIRA User)
Type Bug
Environment

Swift 4.2

Additional Detail from JIRA
Votes 0
Component/s Standard Library
Labels Bug
Assignee None
Priority Medium

md5: 4302353e56be343f06108cc521c4ca83

relates to:

  • SR-5105 inconsistent index of Data

Issue Description:

I've ran into so many `EXC_BAD_INSTRUCTION` crashes because I constantly forget how unnatural slice ranges are in Swift. When I create a slice, I actually expect a window into the array starting with index 0 again, like golang.org does it. This feels natural and intuitive. It makes absolutely no sense to retain index numbers from an array that has nothing to do with the current view of the data, so to actually get the "correct" behaviour you have to wrap it again and convert the slice back.

let data = Data(bytes: [0x1, 0x2, 0x3, 0x4])
let slice = data[1...2]

slice.startIndex // 1
slice.endIndex // 3
slice.count // 2

// This is how it should work
let saneSlice = Data(data[1...2])

saneSlice.startIndex // 0
saneSlice.endIndex // 2
saneSlice.count // 2
@belkadan
Copy link
Contributor

belkadan commented Oct 1, 2018

Changing this behavior would mean that you couldn't use any of the Collection algorithms on a Slice and get back an index you could use in the original value, and slices of slices would be even worse. For any collection other than Array and Data, this isn't a problem, because there's not an obvious "start index", and just having Array and Data behave differently would probably be worse.

@airspeedswift, anything to add?

@swift-ci
Copy link
Collaborator Author

swift-ci commented Oct 2, 2018

Comment by Erik Aigner (JIRA)

For now I implemented my own `Buffer` type to do proper slicing and avoid copying data.

Maybe a similar type would be helpful for data manipulation in Swift?

//
//  Buffer.swift
//  Git
//
//  Created by Erik Aigner on 02.10.18.
//  Copyright © 2018 bytieful. All rights reserved.
//
import Foundation


/// Buffer maintains a buffer without copy-on-write semantics.
public struct Buffer {
    
    private let ref: PointerRef
    private let range: Range<Int>
    
    public init(data: Data) {
        let storage = UnsafeMutableBufferPointer<UInt8>.allocate(capacity: data.count)
        let n = data.copyBytes(to: storage)
        
        assert(n == data.count)
        
        self.init(ref: PointerRef(storage), range: 0..<data.count)
    }
    
    private init(ref: PointerRef, range: Range<Int>) {
        self.range = range
        self.ref = ref
    }
    
    public var count: Int {
        return range.count
    }
    
    public subscript(_ i: Int) -> UInt8 {
        return ref.buf[range.startIndex + i]
    }
    
    /// Returns a slice.
    /// The new buffer slice start index is 0 and count is the range width.
    public subscript(_ range: Range<Int>) -> Buffer {
        let start = self.range.startIndex + range.startIndex
        let end = start + range.count
        return Buffer(ref: ref, range: start..<end)
    }
    
    public func data() -> Data {
        guard let baseAddress = ref.buf.baseAddress else {
            return Data()
        }
        return Data(bytes: baseAddress + range.startIndex, count: range.count)
    }
}


/// Used to automatically release a buffer pointer
/// if all referencing objects cease to exist.
private class PointerRef {
    
    let buf: UnsafeMutableBufferPointer<UInt8>
    
    init(_ buf: UnsafeMutableBufferPointer<UInt8>) {
        self.buf = buf
    }
    
    deinit {
        buf.deallocate()
    }
}

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. standard library Area: Standard library umbrella
Projects
None yet
Development

No branches or pull requests

2 participants