Re: [swift-evolution] [Pitch] deprecating ManagedBufferPointer

[Erik Eckstein via swift-evolution]

> On Oct 19, 2016, at 3:28 PM, Alexis <abeingess...@apple.com> wrote:
> 
> A bit late to this game, because I didn’t fully understand the “point” of 
> ManagedBuffer(Pointer). After a good week of messing around with these in 
> Dictionary/Array/String, I now have Opinions.
> 
> I agree ManagedBufferPointer is largely unnecessary. However it’s seeming a 
> lot like ManagedBuffer (and its equivalents) are suboptimal for the standard 
> library’s purposes too!
> 
> In particular, pretty much every one of these buffers that I see wants to be 
> a subclass of some NS* collection so that it can be toll-free bridged into 
> objective C. This means that all those types are forced to directly drop down 
> to allocWithTailElems, rather than using a nice abstraction that does it for 
> them. Array does this right now, and I’ve got a PR up for review that’s doing 
> the same thing to the HashedCollections. It’s an outstanding bug that String 
> isn’t doing this (forcing its buffer to be wrapped in another class to be 
> bridged).
> 
> I don’t really feel any pain from directly using allocWithTailElems, it’s a 
> great API. It just leaves me at a loss for when I’d reach for ManagedBuffer 
> at all, as it’s very limited.

I think we can implement a replacement for ManagedBufferPointer. But it would 
have a different API. Something like that:

public struct ManagedBufferPointer2 {

  public init(minimumCapacity: Int,
  initWith factory: (StorageClass) throws -> ()
  ) rethrows {
buffer = Builtin.allocWithTailElems_1(
 StorageClass.self, minimumCapacity._builtinWordValue, Element.self)
try factory(buffer)
  }
  // ...
  let buffer: StorageClass
}


> 
> 
>> On Oct 13, 2016, at 3:11 PM, Erik Eckstein via swift-evolution 
>> <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:
>> 
>> I created a proposal: https://github.com/apple/swift-evolution/pull/545 
>> <https://github.com/apple/swift-evolution/pull/545>
>> 
>>> On Oct 11, 2016, at 11:32 PM, Dave Abrahams via swift-evolution 
>>> <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:
>>> 
>>> 
>>> on Tue Oct 11 2016, Károly Lőrentey <swift-evolution@swift.org 
>>> <mailto:swift-evolution@swift.org>> wrote:
>>> 
>>>> +1
>>>> 
>>>> ManagedBuffer has been really useful a couple of times, but I never
>>>> found a use for ManagedBufferPointer. I can’t even say I’m entirely
>>>> sure what need it was originally designed to fulfill.
>>> 
>>> The real need is/was to be able to do the same kind of storage
>>> management in classes not derived from ManagedBuffer.  This can be
>>> important for bridging, where the buffers of various native swift
>>> containers need to be derived from, e.g., NSString or NSArray.  That is,
>>> however, an extremely stdlib-specifc need.
>>> 
>>> 
>>>>> On 2016-10-11, at 00:12, Erik Eckstein via swift-evolution
>>>> <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:
>>>>> 
>>>>> The purpose of ManagedBufferPointer is to create a buffer with a custom 
>>>>> class-metadata to be able
>>>> to implement a custom deinit (e.g. to destroy the tail allocated elements).
>>>>> It was used in Array (before I replaced it with the new 
>>>>> tail-allocated-array-built-ins). But now
>>>> it’s not used anymore in the standard library.
>>>>> 
>>>>> As a replacement for ManagedBufferPointer one can just derive a class 
>>>>> from ManagedBuffer and implement the deinit in the derived class.
>>>>> 
>>>>> final class MyBuffer : ManagedBuffer<MyHeader, MyElements> {
>>>>>  deinit {
>>>>>// do whatever needs to be done
>>>>>  }
>>>>> }
>>>>> 
>>>>> // creating MyBuffer:
>>>>> let b = MyBuffer.create(minimumCapacity: 27, makingHeaderWith: { myb in 
>>>>> return MyHeader(...) })
>>>>> 
>>>>> IMO ManagedBuffer is much cleaner than ManagedBufferPointer (it doesn’t 
>>>>> need this custom
>>>> bufferClass to be passed to the constructor). Also ManagedBufferPointer 
>>>> doesn’t use SIL
>>>> tail-allocated arrays internally. Although this is not something visible 
>>>> to the programmer, it makes
>>>> life easier for the compiler.
>>>>> 
>>>>> So I suggest that we deprecate Ma

Re: [swift-evolution] [Pitch] deprecating ManagedBufferPointer

[Erik Eckstein via swift-evolution]

I created a proposal: https://github.com/apple/swift-evolution/pull/545 
<https://github.com/apple/swift-evolution/pull/545>

> On Oct 11, 2016, at 11:32 PM, Dave Abrahams via swift-evolution 
> <swift-evolution@swift.org> wrote:
> 
> 
> on Tue Oct 11 2016, Károly Lőrentey <swift-evolution@swift.org 
> <mailto:swift-evolution@swift.org>> wrote:
> 
>> +1
>> 
>> ManagedBuffer has been really useful a couple of times, but I never
>> found a use for ManagedBufferPointer. I can’t even say I’m entirely
>> sure what need it was originally designed to fulfill.
> 
> The real need is/was to be able to do the same kind of storage
> management in classes not derived from ManagedBuffer.  This can be
> important for bridging, where the buffers of various native swift
> containers need to be derived from, e.g., NSString or NSArray.  That is,
> however, an extremely stdlib-specifc need.
> 
> 
>>> On 2016-10-11, at 00:12, Erik Eckstein via swift-evolution
>> <swift-evolution@swift.org> wrote:
>>> 
>>> The purpose of ManagedBufferPointer is to create a buffer with a custom 
>>> class-metadata to be able
>> to implement a custom deinit (e.g. to destroy the tail allocated elements).
>>> It was used in Array (before I replaced it with the new 
>>> tail-allocated-array-built-ins). But now
>> it’s not used anymore in the standard library.
>>> 
>>> As a replacement for ManagedBufferPointer one can just derive a class from 
>>> ManagedBuffer and implement the deinit in the derived class.
>>> 
>>> final class MyBuffer : ManagedBuffer<MyHeader, MyElements> {
>>>  deinit {
>>>// do whatever needs to be done
>>>  }
>>> }
>>> 
>>> // creating MyBuffer:
>>> let b = MyBuffer.create(minimumCapacity: 27, makingHeaderWith: { myb in 
>>> return MyHeader(...) })
>>> 
>>> IMO ManagedBuffer is much cleaner than ManagedBufferPointer (it doesn’t 
>>> need this custom
>> bufferClass to be passed to the constructor). Also ManagedBufferPointer 
>> doesn’t use SIL
>> tail-allocated arrays internally. Although this is not something visible to 
>> the programmer, it makes
>> life easier for the compiler.
>>> 
>>> So I suggest that we deprecate ManagedBufferPointer.
>>> 
>>> Erik
>>> ___
>>> swift-evolution mailing list
>>> swift-evolution@swift.org
>>> https://lists.swift.org/mailman/listinfo/swift-evolution
>> 
>> ___
>> swift-evolution mailing list
>> swift-evolution@swift.org <mailto:swift-evolution@swift.org>
>> https://lists.swift.org/mailman/listinfo/swift-evolution 
>> <https://lists.swift.org/mailman/listinfo/swift-evolution>
>> 
> 
> -- 
> -Dave
> 
> ___
> swift-evolution mailing list
> swift-evolution@swift.org <mailto:swift-evolution@swift.org>
> https://lists.swift.org/mailman/listinfo/swift-evolution 
> <https://lists.swift.org/mailman/listinfo/swift-evolution>
___
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

[swift-evolution] [Pitch] deprecating ManagedBufferPointer

[Erik Eckstein via swift-evolution]

The purpose of ManagedBufferPointer is to create a buffer with a custom 
class-metadata to be able to implement a custom deinit (e.g. to destroy the 
tail allocated elements).
It was used in Array (before I replaced it with the new 
tail-allocated-array-built-ins). But now it’s not used anymore in the standard 
library.

As a replacement for ManagedBufferPointer one can just derive a class from 
ManagedBuffer and implement the deinit in the derived class.

final class MyBuffer : ManagedBuffer {
  deinit {
// do whatever needs to be done
  }
}

// creating MyBuffer:
let b = MyBuffer.create(minimumCapacity: 27, makingHeaderWith: { myb in return 
MyHeader(...) })

IMO ManagedBuffer is much cleaner than ManagedBufferPointer (it doesn’t need 
this custom bufferClass to be passed to the constructor). Also 
ManagedBufferPointer doesn’t use SIL tail-allocated arrays internally. Although 
this is not something visible to the programmer, it makes life easier for the 
compiler.

So I suggest that we deprecate ManagedBufferPointer.

Erik___
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

Re: [swift-evolution] [Pitch] Eliminate `ManagedProtoBuffer`

[Erik Eckstein via swift-evolution]

> This is now part of a pull - https://github.com/apple/swift-evolution/pull/437
> 
> The discussion for the four items in the pull ran here 
> (http://thread.gmane.org/gmane.comp.lang.swift.evolution/23093), though there 
> was not enough feedback. But given the deadline for breaking changes, I 
> decided to put the proposal together anyway.

OK. How do you like to proceed with that? I'm fine to handle this as part of 
#437 as long as it goes through quickly. If it's accepted, do you want to do 
the implementation? Otherwise I can do it (the removal of ManagedProtoBuffer). 
Roman could help with the `unsafeAddressOf` part.


> On Jul 19, 2016, at 12:17 AM, Charlie Monroe  
> wrote:
> 
> 
>> On Jul 19, 2016, at 8:49 AM, Brent Royal-Gordon  
>> wrote:
>> 
>>> On Jul 18, 2016, at 10:05 PM, Charlie Monroe  
>>> wrote:
>>> 
 I haven't used `ManagedBuffer`, but would it make sense to change the 
 signature of `initialHeader` to `@noescape (elements: 
 UnsafeMutablePointer, capacity: Int) -> Header` and then 
 effectively run it inside a `withUnsafeMutablePointerToElements()` call? 
 That would prevent access to the uninitialized `header` field while also 
 allowing us to eliminate the `ManagedProtoBuffer` type.
>>> 
>>> Wouldn't this disallow access to the capacity field? That's as well defined 
>>> on ManagedProtoBuffer and AFAIK can be accessed during the initialization.
>> 
>> Yes, which is why I suggested it should be passed in to the `initialHeader` 
>> closure too. (It's read-only anyway, so there's no loss of capability.) What 
>> it *does* prevent access to is `withUnsafeMutablePointerToHeader`, but I'm 
>> not sure how that method is supposed to work before the header's been 
>> initialized anyway.
> 
> I'd personally just remove the ManagedProtoBuffer and note in the 
> documentation of ManagedBuffer.create that you should not read the header 
> from within the block. I think it's a similar programming error as indexing 
> out of bounds which is also documented to be a programming error.

I agree.


> 
> I'm not sure whether we could simply eliminate ManagedBuffer altogether since 
> looking at https://github.com/search?l=swift=ManagedBuffer=Code the 
> only places this is used are generally just forks of apple/swift and I 
> haven't found a single use ManagedBuffer.create in the entire stdlib, all of 
> the Array code uses ManagedBufferPointer directly and initializes it with the 
> unsafeBufferObject, which is some subclass of _ContiguousArrayStorageBase...
> 
>> 
>> -- 
>> Brent Royal-Gordon
>> Architechies
>> 
> 

___
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

[swift-evolution] [Pitch] Eliminate `ManagedProtoBuffer`

[Erik Eckstein via swift-evolution]

The reason why `ManagedProtoBuffer` (the base class of `ManagedBuffer`) exists 
is to give the users an extra bit of type safety inside of the closure 
`initialHeader` passed to `ManagedBuffer.create()`.

public class ManagedBuffer
  : ManagedProtoBuffer {

  /// Create a new instance of the most-derived class, calling
  /// `initialHeader` on the partially-constructed object to
  /// generate an initial `Header`.
  public final class func create(
minimumCapacity: Int,
initialHeader: @noescape (ManagedProtoBuffer) throws -> 
Header
  ) rethrows -> ManagedBuffer {
// ...
  }
}

This closure receives the `ManagedBuffer` instance and returns the initial 
value that is stored in the buffer (the header part of the buffer).  We are 
passing the `ManagedBuffer` as `ManagedProtoBuffer` to prevent the closure from 
reading the uninitialized `value` property. This property is defined in 
`ManagedBuffer` but not in `ManagedProtoBuffer`.

  public final var header: Header {
// ...
  }

This extra bit of safety requires the existence of `ManagedProtoBuffer`, which 
complicates the API.
The name may also lead to some confusion with Google's Protocol Buffers project.

This proposal suggests removing `ManagedProtoBuffer` in order to simplify the 
API.
It means that  `ManagedBuffer` would not be derived from `ManagedProtoBuffer` 
and all methods from `ManagedProtoBuffer` would be moved to `ManagedBuffer`.
The closure `initialHeader` would receive a  `ManagedBuffer` instead of a 
`ManagedProtoBuffer`.

public class ManagedBuffer {

  /// Create a new instance of the most-derived class, calling
  /// `initialHeader` on the partially-constructed object to
  /// generate an initial `Header`.
  public final class func create(
minimumCapacity: Int,
initialHeader: @noescape (ManagedBuffer) throws -> Header
  ) rethrows -> ManagedBuffer {
// ...
  }
}

Also `ManagedBufferPointer`'s init function (the second occurrence of  
`ManagedProtoBuffer`) would receive a  `ManagedBuffer` instead of a 
`ManagedProtoBuffer`:

  /// Manage the given `buffer`.
  ///
  /// - Note: It is an error to use the `header` property of the resulting
  ///   instance unless it has been initialized.
  internal init(_ buffer: ManagedBuffer) {
_nativeBuffer = Builtin.castToNativeObject(buffer)
  }


Motivation
==
The removal of `ManagedProtoBuffer` would simplify the API and avoids confusion 
with Google's Protocol Buffers.

Alternatives

*) Leave as is.
*) Rename the class to avoid the "confusion"-problem.

___
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution