[swift-evolution] SE-184 Improved Pointers

[Taylor Swift]

On Wed, Aug 9, 2017 at 2:34 AM, Andrew Trick <atrick at apple.com> wrote:

>
> On Aug 8, 2017, at 11:10 PM, Taylor Swift <kelvin13ma at gmail.com> wrote:
>
>
> On Wed, Aug 9, 2017 at 1:51 AM, Andrew Trick <atrick at apple.com> wrote:
>
>>
>> On Aug 8, 2017, at 8:44 PM, Taylor Swift <kelvin13ma at 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.
>>
>>
> 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.
>
>
> If it’s copying from a buffer it should not take a count, if it’s copying
> from a pointer it obviously needs to take a count. What I mean by the two
> versions being named consistently is simply that they’re both named
> `copyBytes`. That really isn’t important though. The overflow/underflow
> semantics being consistent are important.
>
> (Incidentally, the reason “bytes” needs to be in the somewhere name is
> because this method isn’t capable of copying nontrivial values)
>
> Should the methods that don’t deal with raw buffers also be modified to
> use the source argument (i.e. UnsafeMutableBufferPointer.
> initialize(from:))?
>
>
> I’m not sure what you mean by this. It also allows the destination to be
> larger than the source. Initializing from a sequence does not trap on
> overflow because we can’t guarantee the size of the sequence ahead of time.
> When I talk about consistent overflow/underflow semantics, I’m only talking
> about initializing one unsafe buffer/pointer from another unsafe
> buffer/pointer.
>
> 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.
>
>
> Mainly because partial initialization is more expected than dropping data
> on the floor. Ultimately, this should be whatever typical developers would
> expect the behavior to be. I would be very hesitant to change the behavior
> now though.
>
> -Andy
>

The problem is I would expect to be able to safely call deinitialize() and
friends after calling initialize(from:). If Element is a class type and
initialize doesn’t fill the entire buffer range, calling deinitialize()
will crash. That being said, since copy(from:bytes:) and copyBytes(from:)
don’t do any initialization and have no direct counterparts in
UnsafeMutableBufferPointer, it’s okay if they have different behavior than
the other methods.


>
> —
>>
>> 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 at apple.com> wrote:
>>
>>>
>>> On Aug 8, 2017, at 8:29 PM, Taylor Swift <kelvin13ma at gmail.com> wrote:
>>>
>>>
>>>
>>> On Tue, Aug 8, 2017 at 11:24 PM, Andrew Trick <atrick at apple.com> wrote:
>>>
>>>>
>>>> On Aug 8, 2017, at 6:51 PM, Taylor Swift <kelvin13ma at gmail.com> wrote:
>>>>
>>>>
>>>>
>>>> On Tue, Aug 8, 2017 at 9:38 PM, Andrew Trick <atrick at 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.
>>>>>
>>>>> Runtime bug: https://bugs.swift.org/browse/SR-5664
>>>>>
>>>>>
>>>> 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
>>>
>>>
>>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-evolution/attachments/20170809/f1e5bc06/attachment.html>