[Review #2] SE-0101: Reconfiguring sizeof and related functions into a unified MemoryLayout struct


(Chris Lattner) #1

Hello Swift community,

The second review of "SE-0101: Reconfiguring sizeof and related functions into a unified MemoryLayout struct" begins now and runs through July 19. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0101-standardizing-sizeof-naming.md

Reviews are an important part of the Swift evolution process. All reviews should be sent to the swift-evolution mailing list at

  https://lists.swift.org/mailman/listinfo/swift-evolution

or, if you would like to keep your feedback private, directly to the review manager.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and contribute to the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

  * What is your evaluation of the proposal?
  * Is the problem being addressed significant enough to warrant a change to Swift?
  * Does this proposal fit well with the feel and direction of Swift?
  * If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
  * How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

More information about the Swift evolution process is available at

  https://github.com/apple/swift-evolution/blob/master/process.md

Thank you,

-Chris Lattner
Review Manager


(Scott James Remnant) #2

+1

Reviewed the previous version, I find this much cleaner and very Swift-y.

Scott


(Gwynne Raskind) #3

  * What is your evaluation of the proposal?

Very strong +1; cleaning up the global namespace, addressing confusion with usage of these functions, and promoting the concept of the low-level attributes of a type being related to that type (rather than being arbitrarily global truths), are all significant wins for readability, discoverability, and conceptual clarity.

  * Is the problem being addressed significant enough to warrant a change to Swift?

Definitively. The sizeof() family of functions are neither commonly enough used nor fundamental enough to idiomatic Swift to belong in the global namespace, and having them there adds potentially dangerous confusion for users coming from C, C++, and Objective-C. In particular, "sizeof(T)" almost definitely doesn’t mean what a newcomer to Swift expects, but when encountering "MemoryLayout<T>.size" one is considerably more likely to have at least noticed the documentation of what it means and the fact that ".stride" exists.

  * Does this proposal fit well with the feel and direction of Swift?

Very much so. To me, the syntax proposed here is simpler, clearer, and much more in keeping with OO design.

  * If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

The closest equivalent I can think of comes from C++, per std::numeric_limits<T>, std::pointer_traits<T>, std::allocator_traits<T>, etc. While C++ is being typically verbose in its syntax, it successfully represents the type information in object-oriented and unambiguous fashion, and the sheer verbosity aside, I’ve always liked this representation. This proposal is substantially similar in form and at a quick glance at my own Swift code, it adds similar clarity to the intent the code expresses.

  * How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

A quick reading.

-- Gwynne Raskind
More magic than a mere signature can contain

···

On Jul 12, 2016, at 18:53, Chris Lattner <clattner@apple.com> wrote:


(David Owens II) #4

What is your evaluation of the proposal?
I’m not in favor of the majority of this proposal (I do support getting rid of the *Value versions of the functions). Of the motivations, I find this one to be the most awkward:

Refactoring this proposal to use a single namespace increases discoverability, provides a single entry point for related operations, and enables future expansions without introducing further freestanding functions.

While technically storing stuff under a new type creates a level of indirection that we may refer to a namespace, it does so in a way that I’m not terribly convinced is even conceptually correct. A new MemoryLayout struct implies there is actually a type we are creating here, but that’s not the case: we are simply using the type as a hack to get a namespace because Swift lacks support for that notion.

If we must have a struct to achieve this, than at the very least, can’t we move all of the generics onto the function call itself?

public struct MemoryLayout {
    public static func sizeof<T>(_ x: T.Type) -> Int { return Swift.sizeof(x) }
// ...
}

Further, I find this change to just make readability suffer. Here’s is a random snippet found from a GitHub search:

let data = NSData(bytes: &bytes, length:sizeof(CChar) * bytes.count)
let data = NSData(bytes: &bytes, length:MemoryLayout<CChar>.size * bytes.count)

That’s a lot more typing for what?

let data = NSData(bytes: &bytes, length:MemoryLayout.sizeof(CChar) * bytes.count)

The above actually satisfies the concern, which seems to be primarily that the functions are top-level functions instead of namespaced.

Is the problem being addressed significant enough to warrant a change to Swift?
No. Like mentioned in the proposal, these are already low-use terms. As such, there is a much prior art for these to turn up search results easily for them and their concept apply across multiple languages.

Does this proposal fit well with the feel and direction of Swift?
No. The heart of the proposal seems to be a workaround for the lack of namespacing in Swift.

If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
Yes, those languages have better readability around these terms.

How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
I’ve read the proposal, looked up code snippets, and tested my own implementation within playgrounds.

-David

···

On Jul 12, 2016, at 4:53 PM, Chris Lattner <clattner@apple.com> wrote:

Hello Swift community,

The second review of "SE-0101: Reconfiguring sizeof and related functions into a unified MemoryLayout struct" begins now and runs through July 19. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0101-standardizing-sizeof-naming.md

Reviews are an important part of the Swift evolution process. All reviews should be sent to the swift-evolution mailing list at

  https://lists.swift.org/mailman/listinfo/swift-evolution

or, if you would like to keep your feedback private, directly to the review manager.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and contribute to the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

  * What is your evaluation of the proposal?
  * Is the problem being addressed significant enough to warrant a change to Swift?
  * Does this proposal fit well with the feel and direction of Swift?
  * If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
  * How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

More information about the Swift evolution process is available at

  https://github.com/apple/swift-evolution/blob/master/process.md

Thank you,

-Chris Lattner
Review Manager

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


(Brent Royal-Gordon) #5

  * What is your evaluation of the proposal?

I think grouping these into a type is a sensible approach, but I don't like that it allows the creation of meaningless MemoryLayout instances. The simplest fix would be to make `MemoryLayout` an empty enum instead of an empty struct. This would convey that no MemoryLayout instances do or can exist.

However, I'm also not really a fan of the way this reads. `MemoryLayout<Int>` is an unnatural way to access this functionality, quite different from how generics are typically used. The heavy use of type members, with instance behavior as a complete afterthought, is very unusual. If we are serious about use sites being the most important thing, we ought to be concerned about these use sites.

I would prefer to see an instance-centric version of this design, with use sites along the lines of:

  MemoryLayout(of: Int.self).size
  let buffer = UnsafeRawPointer.allocate(bytes: MemoryLayout(of: Int.self).stride * count)

If the problem is that it would sometimes misbehave—for instance, when someone tries to construct a MemoryLayout instance from a `type(of:)` call—then we should make it behave correctly, or at least consider it a bug to be fixed eventually.

(Incidentally, I notice that the ABI documentation lists the size, alignment, and stride as part of the type's value witness table. <https://github.com/apple/swift/blob/master/docs/ABI.rst#common-metadata-layout> Would it make sense to think of this as exposing the value witness table as a user-visible type? How might that be different from what's being proposed here?)

  * Is the problem being addressed significant enough to warrant a change to Swift?

Yes. We need to lock this down.

  * Does this proposal fit well with the feel and direction of Swift?

See my comment above about how it reads.

  * If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

Well, it *is* more coherent and less magical than the C family.

  * How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Quick reading, but I've chimed in during previous discussions (though not in this latest round—family duties have kept me from my mail client).

···

On Jul 12, 2016, at 4:53 PM, Chris Lattner <clattner@apple.com> wrote:

--
Brent Royal-Gordon
Architechies


(Tino) #6

I share the minor concerns that have been expressed so far.
My first thought on this topic ("MyStruct.size") had a obvious flaw, but I wouldn't fear name collisions with something like "MyStruct.memoryLayout.size".
This one most likely has issues as well, but imho they aren't that obvious and deserve being discussed in the alternatives-section (or a short answer here to wake up my tired mind :wink:

- Tino


(Shawn Erickson) #7

Strong +1 for me as well, can't summarize it better then Gwynne's email

···

On Tue, Jul 12, 2016 at 8:48 PM Gwynne Raskind via swift-evolution < swift-evolution@swift.org> wrote:

> On Jul 12, 2016, at 18:53, Chris Lattner <clattner@apple.com> wrote:
> * What is your evaluation of the proposal?

Very strong +1; cleaning up the global namespace, addressing confusion
with usage of these functions, and promoting the concept of the low-level
attributes of a type being related to that type (rather than being
arbitrarily global truths), are all significant wins for readability,
discoverability, and conceptual clarity.

> * Is the problem being addressed significant enough to warrant a
change to Swift?

Definitively. The sizeof() family of functions are neither commonly enough
used nor fundamental enough to idiomatic Swift to belong in the global
namespace, and having them there adds potentially dangerous confusion for
users coming from C, C++, and Objective-C. In particular, "sizeof(T)"
almost definitely doesn’t mean what a newcomer to Swift expects, but when
encountering "MemoryLayout<T>.size" one is considerably more likely to have
at least noticed the documentation of what it means and the fact that
".stride" exists.

> * Does this proposal fit well with the feel and direction of Swift?

Very much so. To me, the syntax proposed here is simpler, clearer, and
much more in keeping with OO design.

> * If you have used other languages or libraries with a similar
feature, how do you feel that this proposal compares to those?

The closest equivalent I can think of comes from C++, per
std::numeric_limits<T>, std::pointer_traits<T>, std::allocator_traits<T>,
etc. While C++ is being typically verbose in its syntax, it successfully
represents the type information in object-oriented and unambiguous fashion,
and the sheer verbosity aside, I’ve always liked this representation. This
proposal is substantially similar in form and at a quick glance at my own
Swift code, it adds similar clarity to the intent the code expresses.

> * How much effort did you put into your review? A glance, a quick
reading, or an in-depth study?

A quick reading.

-- Gwynne Raskind
More magic than a mere signature can contain

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


(Dave Abrahams) #8

  * What is your evaluation of the proposal?

I think grouping these into a type is a sensible approach, but I don't
like that it allows the creation of meaningless MemoryLayout
instances. The simplest fix would be to make `MemoryLayout` an empty
enum instead of an empty struct. This would convey that no
MemoryLayout instances do or can exist.

+1.

However, I'm also not really a fan of the way this
reads. `MemoryLayout<Int>` is an unnatural way to access this
functionality, quite different from how generics are typically
used. The heavy use of type members, with instance behavior as a
complete afterthought, is very unusual. If we are serious about use
sites being the most important thing, we ought to be concerned about
these use sites.

This design is specifically optimized to make use-sites clear and
noise-free.

The fact that using the type as a generic parameter very clearly states
that we're asking about the size of the type, and not the metatype, also
helps.

Lastly, any other design is more verbose, requiring ".self" on the
passed metatype.

I would prefer to see an instance-centric version of this design, with use sites along the lines of:

  MemoryLayout(of: Int.self).size
  let buffer = UnsafeRawPointer.allocate(bytes: MemoryLayout(of: Int.self).stride * count)

I don't think that objectively reads better than:

   let buffer = UnsafeRawPointer.allocate(bytes: MemoryLayout<Int>.stride * count)

I can understand your preference for it as a matter of taste, but I
think the second one is just as understandable and much less
noisy... thus clearer.

If the problem is that it would sometimes misbehave—for instance, when
someone tries to construct a MemoryLayout instance from a `type(of:)`
call—then we should make it behave correctly, or at least consider it
a bug to be fixed eventually.

(Incidentally, I notice that the ABI documentation lists the size,
alignment, and stride as part of the type's value witness
table. <https://github.com/apple/swift/blob/master/docs/ABI.rst#common-metadata-layout>
Would it make sense to think of this as exposing the value witness
table as a user-visible type?

Definitely not.

How might that be different from what's being proposed here?)

This is stable, documented API; the value witness table is not. :slight_smile:

···

on Wed Jul 13 2016, Brent Royal-Gordon <swift-evolution@swift.org> wrote:

On Jul 12, 2016, at 4:53 PM, Chris Lattner <clattner@apple.com> wrote:

  * Is the problem being addressed significant enough to warrant a change to Swift?

Yes. We need to lock this down.

  * Does this proposal fit well with the feel and direction of Swift?

See my comment above about how it reads.

  * If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

Well, it *is* more coherent and less magical than the C family.

  * How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Quick reading, but I've chimed in during previous discussions (though not in this latest round—family duties have kept me from my mail client).

--
Dave


(Dave Abrahams) #9

We're trying to get away from “magic” properties that exist on every
type, I think.

···

on Thu Jul 14 2016, Tino Heth <swift-evolution@swift.org> wrote:

I share the minor concerns that have been expressed so far.
My first thought on this topic ("MyStruct.size") had a obvious flaw, but I wouldn't fear name collisions with something like "MyStruct.memoryLayout.size".
This one most likely has issues as well, but imho they aren't that
obvious and deserve being discussed in the alternatives-section (or a
short answer here to wake up my tired mind :wink:

--
Dave


(Erica Sadun) #10

+1 as well.

/// Accesses the memory layout of `T` through its
/// `size`, `stride`, and `alignment` properties
public enum MemoryLayout {
    /// Returns the contiguous memory footprint of `T`.
    ///
    /// Does not include any dynamically-allocated or "remote"
    /// storage. In particular, `MemoryLayout.size`, when
    /// `T` is a class type, is the same regardless of how many
    /// stored properties `T` has.
    public static var size: Int { return _sizeof(T.self) }
    
    /// For instances of `T` in an `Array`, returns the number of
    /// bytes from the start of one instance to the start of the
    /// next. This is the same as the number of bytes moved when an
    /// `UnsafePointer` is incremented. `T` may have a lower minimal
    /// alignment that trades runtime performance for space
    /// efficiency. The result is always positive.
    public static var stride: Int { return _strideof(T.self) }
    
    /// Returns the default memory alignment of `T`.
    public static var alignment: Int { return _alignof(T.self) }
}

-- E

···

On Jul 13, 2016, at 5:39 PM, Dave Abrahams via swift-evolution <swift-evolution@swift.org> wrote:

on Wed Jul 13 2016, Brent Royal-Gordon <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

I think grouping these into a type is a sensible approach, but I don't
like that it allows the creation of meaningless MemoryLayout
instances. The simplest fix would be to make `MemoryLayout` an empty
enum instead of an empty struct. This would convey that no
MemoryLayout instances do or can exist.

+1.


(Tino) #11

I've a serious aversion against regular-looking constructs with special power as well (from time to time, I find myself thinking that ErrorType and SequenceType are somewhat odd… :wink:
On the other hand:
Is a magic struct better than a magic property?
After all, the size of a type is a property of that type…

Tino

···

Am 15.07.2016 um 23:56 schrieb Dave Abrahams via swift-evolution <swift-evolution@swift.org>:

We're trying to get away from “magic” properties that exist on every
type, I think.


(John McCall) #12

I think grouping these into a type is a sensible approach, but I don't
like that it allows the creation of meaningless MemoryLayout
instances. The simplest fix would be to make `MemoryLayout` an empty
enum instead of an empty struct. This would convey that no
MemoryLayout instances do or can exist.

+1.

+1 as well.

If MemoryLayout were not a generic type, its instances would not necessarily be meaningless; it could be used to lay out memory in a size + alignment-safe way.

You would need to restructure most of the API you've been talking about, though.

John.

···

On Jul 18, 2016, at 8:57 AM, Erica Sadun via swift-evolution <swift-evolution@swift.org> wrote:

On Jul 13, 2016, at 5:39 PM, Dave Abrahams via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:
on Wed Jul 13 2016, Brent Royal-Gordon <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

/// Accesses the memory layout of `T` through its
/// `size`, `stride`, and `alignment` properties
public enum MemoryLayout {
    /// Returns the contiguous memory footprint of `T`.
    ///
    /// Does not include any dynamically-allocated or "remote"
    /// storage. In particular, `MemoryLayout.size`, when
    /// `T` is a class type, is the same regardless of how many
    /// stored properties `T` has.
    public static var size: Int { return _sizeof(T.self) }
    
    /// For instances of `T` in an `Array`, returns the number of
    /// bytes from the start of one instance to the start of the
    /// next. This is the same as the number of bytes moved when an
    /// `UnsafePointer` is incremented. `T` may have a lower minimal
    /// alignment that trades runtime performance for space
    /// efficiency. The result is always positive.
    public static var stride: Int { return _strideof(T.self) }
    
    /// Returns the default memory alignment of `T`.
    public static var alignment: Int { return _alignof(T.self) }
}

-- E

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


(Xiaodi Wu) #13

I think grouping these into a type is a sensible approach, but I don't
like that
* it allows the creation of meaningless MemoryLayoutinstances*. The
simplest fix would be to make `MemoryLayout` an empty
enum instead of an empty struct. This would convey that no
MemoryLayout instances do or can exist.

+1.

+1 as well.

I must be missing something. How would one create MemoryLayout instances
without a public initializer? Why would it matter if stdlib could formally
do so internally, given the plethora of much less harmless things that
stdlib can do internally?

/// Accesses the memory layout of `T` through its

···

On Mon, Jul 18, 2016 at 10:57 AM, Erica Sadun via swift-evolution < swift-evolution@swift.org> wrote:

On Jul 13, 2016, at 5:39 PM, Dave Abrahams via swift-evolution < > swift-evolution@swift.org> wrote:
on Wed Jul 13 2016, Brent Royal-Gordon <swift-evolution@swift.org> wrote:

/// `size`, `stride`, and `alignment` properties
public enum MemoryLayout {
    /// Returns the contiguous memory footprint of `T`.
    ///
    /// Does not include any dynamically-allocated or "remote"
    /// storage. In particular, `MemoryLayout.size`, when
    /// `T` is a class type, is the same regardless of how many
    /// stored properties `T` has.
    public static var size: Int { return _sizeof(T.self) }

    /// For instances of `T` in an `Array`, returns the number of
    /// bytes from the start of one instance to the start of the
    /// next. This is the same as the number of bytes moved when an
    /// `UnsafePointer` is incremented. `T` may have a lower minimal
    /// alignment that trades runtime performance for space
    /// efficiency. The result is always positive.
    public static var stride: Int { return _strideof(T.self) }

    /// Returns the default memory alignment of `T`.
    public static var alignment: Int { return _alignof(T.self) }
}

-- E

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