I’m fine with switching to taking the count from the source, though I think
taking the count from the destination is slightly better because 1) the use
cases I mentioned in the other email, and 2) all the other memorystate
functions use self.count instead of source.count, if they take a source
argument. But being consistent with the raw pointer version is more
important.
Should the methods that don’t deal with raw buffers also be modified to use
the source argument (i.e. UnsafeMutableBufferPointer.initialize(from:))?
Also, was there a reason why UnsafeMutableRawBufferPointer.copyBytes(from:)
uses the source’s count instead of its own? Right now this behavior is
“technically” undocumented behavior (as the public docs haven’t been
updated) so if there was ever a time to change it, now would be it.
···
On Wed, Aug 9, 2017 at 1:51 AM, Andrew Trick <atrick@apple.com> wrote:
On Aug 8, 2017, at 8:44 PM, Taylor Swift <kelvin13ma@gmail.com> wrote:
cool,, as for UnsafeMutableRawBufferPointer.copy(from:bytes:), I cannot
find such a function anywhere in the API. There is copyBytes(from:)
<https://developer.apple.com/documentation/swift/unsafemutablerawbufferpointer/2635415-copybytes>,
but the documentation is messed up and mentions a nonexistent count:
argument over and over again. The documentation also doesn’t mention what
happens if there is a length mismatch, so users are effectively relying on
an implementation detail. I don’t know how to best resolve this.We currently have `UnsafeMutableRawBufferPointer.copyBytes(from:)`. I
don’t think your proposal changes that. The current docs refer to the
`source` parameter, which is correct. Docs refer to the parameter name, not
the label name. So `source.count` is the size of the input. I was pointing
out that it has the semantics: `debugAssert(source.count <= self.count)`.Your proposal changes `UnsafeRawPointer.copyBytes(from:count:)` to
`UnsafeRawPointer.copy(from:bytes:)`. Originally we wanted to those API
names to match, but I’m fine with your change. What is more important is
that the semantics are the same as `copyBytes(from:)`. Furthermore, any new
methods that you add that copy into a raw buffer (e.g.
initializeMemory(as:from:count:)) should have similar behavior.
—
Another thing. The initialization methods that you’re adding to
`UnsafeRawPointer` and `UnsafeRawBufferPointer` should return typed
`UnsafePointer<Element>` and `UnsafeBufferPointer<Element>` respectively.
I’ll fix that once the current pending edit
<https://github.com/apple/swift-evolution/pull/741> gets merged.
Thanks,
-Andy
On Tue, Aug 8, 2017 at 11:33 PM, Andrew Trick <atrick@apple.com> wrote:
On Aug 8, 2017, at 8:29 PM, Taylor Swift <kelvin13ma@gmail.com> wrote:
On Tue, Aug 8, 2017 at 11:24 PM, Andrew Trick <atrick@apple.com> wrote:
On Aug 8, 2017, at 6:51 PM, Taylor Swift <kelvin13ma@gmail.com> wrote:
On Tue, Aug 8, 2017 at 9:38 PM, Andrew Trick <atrick@apple.com> wrote:
> UnsafeMutableRawBufferPointer.allocate(bytes:alignedTo:)
Well, I think it's somewhat ridiculous for users to write this every
time they allocate a buffer:`UnsafeMutableRawBufferPointer.allocate(bytes: size, alignedTo:
MemoryLayout<UInt>.alignment)`If anyone reading the code is unsure about the Swift API's alignment
guarantee, it's trivial to check the API docs.You could introduce a clearly documented default `alignedTo`
argument. The reason I didn't do that is that the runtime won't
respect it anyway. But I think it would be fair to go ahead with the
API and file a bug against the runtime.Default argument of MemoryLayout<Int>.alignment is the way to go but
as you said i don’t know if that is actually allowed/works. An alternative
is to have two allocate methods each, one that takes an alignment argument
and one that doesn’t (and aligns to pointer alignment) but that feels
inelegant. Default arguments would be better.Default argument makes sense to me too. Then the raw buffer pointer and
regular raw pointer APIs can be consistent with each other.yikes i was not aware of this. I don’t think it’s bad enough to warrant
dropping the argument like with deallocate(capacity:) but I can imagine
bad things happening to code that crams extra inhabitants into pointers.If we ever need to do pointer adjustment during deallocation to
accommodate alignment, then I think the Swift runtime can track that. I see
no reason to muddy the UnsafeRawPointer API with it. So, I agree with your
proposed change to drop `alignedTo` there.-Andy
oh lol I was talking about assuming the pointer returned by
allocate(bytes:alignedTo:) is a multiple of alignedTo. Some code might
be relying on the last few bits of the pointer being zero; i.e. sticking
bit flags there like how some implementations store the red/black color
information in a red-black tree node.Oh, sure. But I think it will be easy to fix the runtime. We could
probably do it before the proposal is accepted if necessary.
-Andy