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-13486] _withUnsafeMutableBufferPointerIfSupported/withContiguousMutableStorageIfAvailable semantics and docs #55928

Open
dabrahams opened this issue Sep 1, 2020 · 3 comments
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. standard library Area: Standard library umbrella

Comments

@dabrahams
Copy link
Collaborator

Previous ID SR-13486
Radar rdar://problem/68296101
Original Reporter @dabrahams
Type Bug
Additional Detail from JIRA
Votes 0
Component/s Standard Library
Labels Bug
Assignee None
Priority Medium

md5: 35b6ecbdcb3043b2bac85f7ab2130a33

Issue Description:

These two methods have identical doc comments. If I'm working on the stdlib, how should I understand the difference?

Secondly, the doc comments should start with "Calls," not "Call"

Third, they contradict themselves:

> Call `body(p)`, where `p` is a pointer to the collection's mutable contiguous storage. If no such storage exists, it is first created.

Okay, so mutable contiguous storage is created if none is available, and body is called on it.

> If the collection does not support an internal representation in a form of mutable contiguous storage, `body` is not called and `nil` is returned.

In a collection that doesn't support an internal representation in a form of mutable contiguous storage, obviously no such storage exists, so according to the second sentence it should be created. This is a contradiction, and the inability to clearly specify what the method does is a big part of the reason I originally opposed its inclusion in the library: we clearly don't know what it is.

I think (though I'm guessing) what's actually meant is something more like, “If the collection supports a mutable contiguous storage representation, converts (if necessary) the instance to use that representation and returns the result of passing the storage to `body`; returns `nil` otherwise.“

But note that this description doesn't work for the immutable contiguous storage method on Sequence, because Array implements the operation but can't change its representation in a non-mutating method.

I still think these operations are highly suspect, but at least please clear up their meaning.

Thanks

@typesanitizer
Copy link

@swift-ci create

@swift-ci
Copy link
Collaborator

swift-ci commented Sep 3, 2020

Comment by Kyle Macomber (JIRA)

Can you open a PR with your recommended changes?

@dabrahams
Copy link
Collaborator Author

So kylemacomber (JIRA User) my friend… I tell you there's an API whose intended semantics is not clear to me and that I opposed having in the first place, and you want me to make a recommendation? My recommendation would be to remove the API, seriously. If that's not what you're looking for, presumably you want me to choose some semantics I think would be useful… but then I think you'll probably want to change the name and signature because the semantics I suggest would create contiguous storage if it didn't already exist and would never return nil; i.e. it would have the same basic semantics as Array's withUnsafeMutableBufferPointer, which has always worked, even when the Array had a non-contiguous bridged NSArray backing store.

In other words, if this is somehow going to be fixed by non-source-breaking changes, someone who understands what this is actually supposed to do is going to need to step up.

@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

3 participants