SE-0370: Pointer family initialization improvements and better buffer slices

Hello Swift community,

The review of SE-0370: Pointer family initialization improvements and better buffer slices begins now and runs through August 29th, 2022.

Reviews are an important part of the Swift evolution process. All review feedback should be either on this forum thread or, if you would like to keep your feedback private, directly to the review manager. When emailing the review manager directly, please keep the proposal link at the top of the message and put "SE-0370" in the subject line.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine 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/main/process.md

Thank you,

John McCall
Review Manager

15 Likes

This mostly looks like a batch of great ergonomics improvements, and definitely addresses a number of pain points I've run into with the pointer APIs.

My only concern would be around the naming of update. To me, that carries the connotation of "update/modify the item in the slot", rather than "update the slot to hold a new item", and I actually prefer the old "assign" terminology. I wonder if "replace" might be a better option than both of them; e.g. something like buffer[subrange].replaceElements(with: newElements) (or some variation thereof).

2 Likes

Replace would be fine, too. "Initialize" and "Update" happen to be the words used in TSPL to describe the roles of = (see Assignment Operator). In that nomenclature, you allocate memory, initialize it to a first value, update it some number of times, deinitialize it, then finally deallocate it.

1 Like

Sorry to jump in on the bikeshedding, but what about just..."reassign"? To me this reads even more strongly that an initial assignment to a first value had to have happened as a precondition.* And the word maintains some recognizable continuity with the previous name for the API, and like @Torust mentions it avoids suggesting some sort of in-place modification.

*) By contrast, while the word "update" does in the most literal sense imply that a value had to be there originally, it has the same problem in colloquial use as the word "repeat" in that it sometimes sloppily includes the first of a series of changes ("first update" could mean "first version" or "second version," just like "first repeat" could mean "first iteration" or "second iteration").

2 Likes

How is it not an in-place modification of the memory in question? Why is that suggestion to be avoided?

I'll let @Torust explicate his own point rather than trying to speak for him, but in the briefest terms I think it's to do with clarity about what can be mutated—e.g., if the pointee is of a class type, this operation can reassign what instance of the class is there, rather than just mutating the existing instance.

1 Like

Overall, I think this is a sensible series of improvements.

I continue to think it's regrettable that we'll end up with two "generations" of similar APIs in parallel, which requires an essentially arbitrary disambiguation by argument label because we don't want to overload on return type, even though it is for the very good reason that such overloading can break source. But I accept the reason.

My one concrete suggestion with respect to this issue is that I think we can come up with a slight improvement in the label used for disambiguation that can at least stand on its own as an improvement, even if we didn't have to use it to avoid overloading on the return type:

Recall that we have the Collection method append(contentsOf:), which is actually really nice because it helps us to avoid the ambiguity that one might be appending the entire collection as a single element.

I think we could consider a minor tweak to the proposal that reads more fluently at the call site and also calls back to the method above that we all know and love:

--- initialize(fromElements:)
+++ initialize(fromElementsOf:) // or
+++ initialize(fromContentsOf:)

...and similarly for other APIs being introduced here.

1 Like

The label fromContentsOf is a good suggestion.

1 Like

The potential confusion of reference vs. referent is a basic issue in the language, but it is not an advanced concern. My goal is expressing that the chunk of memory being modified must have a valid, preexisting value. The possible confusion with the referent does no harm here, beyond the bugs that would occur with a normal binding: wherever there would be a referent to modify, the reference must be valid. The word "assign" fails to signal this since it is used elsewhere to describe an initialization.

Yes, exactly that. The distinction I'd draw is I'd expect someObject.update(from: otherObject) to preserve the object identity of someObject, whereas I wouldn't be surprised if someObject.replace(with: otherObject) (or reassign(to:)) reassigned self on someObject.

I'd prefer init(fromContents:). The preposition would be unnecessary because there must also be a container for "contents" to exist. I think this would be in line with the guidance to not add unnecessary prepositions on their own in arguments labels — e.g. x.multiply(with: 5) is arguably better as x.multiply(5). Likewise, the reader can subconsciously insert a preposition, without the argument label being cluttered with extra words.

I’m not sure I understand. The API in question is spelled x.multiplied(by: 5); the preposition isn’t elided as it’s necessary to read fluently at the use site.

3 Likes

I would like to see these new APIs be available for older Swift versions, using technics similar to polyfill in web development.

That is, we can provide an official package (eg. swift-stdlib-compatibility) that implements the new standard library APIs for older Swift (behind #if swift check). Library authors won’t need to judge Swift version everywhere or bear depreciation messages, instead they import StandardLibraryCompatibility and use the new APIs directly.

This will not only help the community adopt new APIs, but also lower the curve when we don’t need to support older Swift versions any more. The author can then drop the dependency and remove all the related imports, without touching the implementation body.

If you check out the ABI section of the proposal, you can see that all of the functions proposed here are usable without deployment restrictions.

1 Like

I believe I was talking about Swift instead of OS. e.g. if the proposal is implemented in Swift 5.8, library authors will need to use #if swift(>= 5.8) checks everywhere at call site to avoid deprecation warnings. This could produce a large number of boilerplates and potential problems, especially for libraries that interoperates with C heavily.

An official solution is worth consideration to ease the transition.

2 Likes

Ah right, sorry.

Some existing and proposed APIs return an (Iterator, Index) tuple.

  • Can tuple labels be added or changed?
    The updated: label doesn't seem correct.

  • Can @discardableResult be added?
    This seems equally useful, whether source is a collection or sequence.


The detailed design might be improved with primary associated types.

  • C: Collection<UInt8>
  • C: Collection<Element>
  • S: Sequence<Element>

Adding @discardableResult is fair. I believe it could also be added to the old API.

The tuple labels can't be added to the old API (it can break some existing source code,) but we could possibly adjust the labels being added. However, I made the API added to Slice identical to their counterparts on U[M][R]BP, so that would mean that some of the "new" API don't get tuple labels, because the older ones they are mimicking do not. Is this the right decision?

Thanks for pointing out the primary associated types. This proposal's writing predated those.

Adding tuple labels shouldn't be source-breaking, should it?

(There is the one narrow exception I can think of where the added labels happen to match labels on the left-hand side of a tuple destructuring expression but in a different order, causing an unexpected tuple shuffle to happen—but that is both unlikely and, if the proper labels are chosen, would only fix existing buggy code.)

I don't think this is a good idea -- the result indicates important failure cases, so it ought to be always checked. (At minimum in a debug assertion, such as assert(unwritten.next() == nil && initialized == target.endIndex).) Marking these functions @discardableResult would encourage ignoring these potential error cases, leading to potential data loss and/or security issues.