[Request for Feedback] Partial Consumption of Fields of NonCopyable Types

Evolution Pitches
1

Hey everyone!

The following is a post asking for feedback from the community on the manner in
which we should allow for partial consumption of noncopyable fields of
noncopyable types. Looking forward to everyone's thoughts.

Introduction

Currently given a var like construct (e.x.: var, inout), Swift does not allow
for a stored field of the type to be partially consumed:

struct E : ~Copyable {}

struct S : ~Copyable {
    var first: E
    var second: Klass
}

var s = S()
let _ = s.e // Error! Cannot partially consume s

Since this applies to inouts, this also applies to stored fields of self in
mutating methods. E.x.:

extension S {
    mutating func doSomething() {
        let _ = self.e // Error! Cannot partially consume self
    }
}

That being said, one can still of course pass the field inout to take the field
out by using the consume operator:

extension S {
    mutating func doSomething() {
        let _ = (consume self).e

        self = S()
    }
}

while this works, it is a significiant reduction in expressivity since one has
to consume /all/ of self causing one to be unable to access the rest of the
fields of self later in the function. Given this reduction in expressivity it is
natural to ask... can we improve this situation by allowing for partial
consumption of self:

extension S {
    mutating func doSomething() {
        let _ = e
        print(k) // I can still print k!
        e = E() // Reinitialize e so self is fully initialized at end of
                // doSomething()
    }
}

We can do this and in fact, the Swift compiler already has support for this,
albeit turned off! The reason that it is turned off is that we realized that
there is design space here that we wanted to explore and that when the
noncopyable proposal went through evolution we labeled partial consumption
explicitly as an extension of the proposal. That being said, lets now go through
the design space together.

Partial Consumption of NonCopyable Types

We consider below a few different axes in the design space:

  1. On types with deinits
  2. When Library Evolution is enabled
  3. When Library Evolution is disabled
  4. In either case, whether or not we should allow for partial consumption only
    in methods.

Partial Consumption on types with Deinits

We ban partial consumption of noncopyable types with deinits since when a field
is partially consumed, we are allowing for the type to be destroyed in
parts. For example:

struct E : ~Copyable
struct S : ~Copyable {
   var first: E
   var second: E
   deinit {}
}

var s = S()
let _ = s.first // s.first is destroyed here
doSomething()
// s.second is destroyed here

Since s here has been partially consumed, we never destroy it all together
implying that we never would call its own deinit implying that we must not allow
it. Note that even though we do not allow this, we still allow for authors in
consuming methods to use the discard operator to turn off the deinit of the
value and then partially deconstruct the value.

Partial Consumption when Library Evolution is enabled

The clear invariant that partial consumption of noncopyable types relies upon is
that all stored fields of the noncopyable type must be accessible in the module
where the partial consumption occurs. Naturally this means that in library
evolution our ability to partially consume types is significantly
limited. Specifically:

  1. Frozen types regardless of access control level can always be partially
    consumed. This includes even frozen types with private fields since even
    though the private field is not available to be used it is still exposed at
    the ABI level.

  2. Public and usableFromInline types can never be partially consumed outside of
    the resilience domain where the type is defined. Since resilience domains are
    today limited to the current module, this means that one could not partially
    consume outside of the current module.

  3. Internal types that are not usableFromInline, private, and fileprivate
    noncopyable types can always have their stored properties partially consumed.

Partial Consumption when Library Evolution is disabled

When we compile without library evolution, from an ABI perspective we have
everything that we need to always partially consume even public types since when
library evolution is disabled all types have a frozen ABI. But we have
additional Source Compatibility constraints to consider: we have always allowed
for authors to convert fields from being stored to computed and back. This
creates source stability issues since:

  1. When we convert a stored property to a computed property, we will be
    replacing a partial liveness use of just one of the value's stored fields to
    a use of the entire value since a computed property takes self as a fully
    live value. E.x.:

    // Library
public struct E : ~Copyable {}
public struct S : ~Copyable {
    var first: E
    var second: E
}

// Executable
let _ = s.first // Invalidates s.first
let _ = s.second // Invalidates s.second

->

// Library
struct S : ~Copyable {
    var first: E
    var second: E { E() }
}

// Executable
let _ = s.first // Invalidates s.first.
let _ = s.second // Uses all of s when calling the getter s.second. Use after free!

  2. When we convert a computed property to a stored property, we introduce a new
    partial invalidation potentially causing later code to stop compiling. E.x.:

    // Library
struct E : ~Copyable {}
struct S : ~Copyable {
   var first: E { E () }
   func doSomething() { }
}

// Executable
let _ = s.first // We call the s.first the getter.
s.doSomething() // Call s.doSomething()

->

// Library
public struct S : ~Copyable {
   var first: E
   func doSomething() { }
}

// Executable
let _ = s.first // Invalidate s.first
s.doSomething() // Error! s is not completely initialized.

These source compatibility concerns imply that even in non-ABI stable libraries
we do not want to allow for public noncopyable types to be partially consumed by
default. This suggests that we may want to add the ability for a library author
to explicitly notate such public types that they are giving up these source
compatibility properties. A natural way to do this is to endow @frozen with
this meaning when applied to public noncopyable types in libraries without
library evolution enabled. As an additional benefit, by extending the meaning of
frozen in this manner, we prevent an additional difference in between Swift when
compiled with/without library evolution enabled: in both language modes, public
noncopyable types can only be partially consumed outside of their current module
if they have @frozen attached.

Partial Consumption outside of Methods

The final axis to consider is whether or not we should be even more restrictive
and only allow for partial consumption of noncopyable types inside methods. The
argument in favor of this approach is that the author of a type has the greatest
understanding of the invariants of the type and the impact of a value being
consumed and thus self being invalid. The argument against this is that the move
checker will prevent any such misuses, e.x.: if one were to call any method on
the partially consumed noncopyable type, we would get an error. So even if a
user of a type made such a mistake, it would never actually result in a valid
program. So we would be giving up expressivity without any real gain.

I would love everyone's thoughts here on the design above and as well any use
cases where one thinks that this may be useful.

5 Likes
2

Could we still allow temporary partial consumption? I.e., the error could be suppressed in your first example so long as we put back s.first to allow the full value to be destroyed?

I don’t think I’d want to reuse @frozen for this. It’s been brought up before to reuse this attribute in non-resilient contexts for a meaning like “this won’t change” (in whatever form) but I would rather not dilute the meaning of @frozen by using it in other contexts. The rules of “what you can do with a frozen type” are subtle and important to get right, and I really wouldn’t want conflicting meanings floating around.

Even if we did restrict this, would people be able to get around it just by working in an extension? If so it really seems to me like we don’t need this restriction, I agree with the pitch text here.

2 Likes
3

Can this restriction be relaxed if the type is “made whole” before the borrow ends? Otherwise one still couldn’t do reasonable things like:

struct Queue<T> : ~Copyable {
  var buf = UnmanagerBuffer<T>()
  mutating func append(_ element: T) {
    buf.append(element)
  }

  // contract: you must free the UnmanagedBuffer yourself
  mutating func drain() -> UnmanagedBuffer<T> {
    // avoid copying or retaining buf
    let buffer = consume buf
    buf = UnmanagedBuffer<T>()
    return buf
  }

  deinit { buf.free() }
}
1 Like
4

That is implementable, I would need to think further. But I do think it does provide some expressivity benefits. See my response to @ksluder below

The really nice thing about using frozen here is that it will make it so that in library evolution and outside of library evolution the source looks exactly the same. We really would like to avoid introducing another manner in which library evolution acts differently than code when library evolution is disabled. Otherwise, we have two options:

  1. We could introduce a new attribute that would only be used when library evolution is disabled and allow for frozen to have its intuitive meaning when library evolution is enabled. I would be very against this since we will have just created a dialect.

  2. We could introduce a new attribute that means this in both modes and artificially restrict what frozen means in library evolution mode. I tend to think that people will find it weird that frozen would not imply partial consumption. It would also be another attribute that people need to learn. Swift already has a lot of attributes, so I am wary of this option.

When I look at both of those options and just adding a meaning to frozen, I tend towards modifying frozen. That being said, if we were to create a new attribute do you have a name suggestion just for the purpose of our discussion?

1 Like
5

You can still do that today by consuming self like I mentioned above:

struct Queue<T> : ~Copyable {
  var buf = UnmanagerBuffer<T>()
  mutating func append(_ element: T) {
    buf.append(element)
  }

  // contract: you must free the UnmanagedBuffer yourself
  mutating func drain() -> UnmanagedBuffer<T> {
    // avoid copying or retaining buf
    let buffer = (consume self).buf
    self = Queue<T>(UnmanagedBuffer<T>())
    return buffer
  }

  deinit { buf.free() }
}

... but I get your larger point that just like it is nice to have the expressivity of not having to reconstruct self. For instance, consider if self's initializers have a bunch of arguments or if self only has non-trivial inits with business logic that you don't want to have to turn off when just trivially reconstructing self.

6

I find this reasoning to be persuasive: if @frozen has a certain consequence in library evolution mode, particularly if it’s something that’s not immediately obvious, it seems like it’d be plausibly less confusing to have the same behavior when not in library evolution mode.

2 Likes
7

I don't think the consume self here would disable the deinit, though. So if buf were a computed property, it'd need to discard self to not accidentally call free on the buffer. Which sort of raises another angle of this partial consume concern: how should it interact with discard?

We handle this partially-invalided / partially-initialized case for types with deinits (i.e., classes) when you return early from an init before fully initializing all of self's fields. Since consuming a field of self is like converting self back into a state where it's not fully initialized, can we make the partial consume work like it does for self in an init?

1 Like
8

What if we instead had syntax to pluck out at once all the members we need to consume, wouldn't that solve the problem in a simple way, without having to complicate language rules with definitions of partially deconstructed values?

Sometimes I wish there was a way to pluck multiple properties of a value into a tuple value with a syntax something like t.(.a, .b, .c.0) for a tuple of type (A, B, A) or t.c.(ca: .0, cb: .1) for the tuple type with labels (ca: A, ca: B). Maybe something like that could be used to pluck out multiple inout bindings at once without an exclusivity violation, as long as the members are distinct in memory?

do { // Future direction?
  inout (a, b, ca) = &t.(.a, .b, .c.a)
}

See: Pitch: `borrow` and `inout` declaration keywords - #16 by pyrtsa

9

I see why this approach is compelling, but I’m not convinced that @frozen truly ends up meaning the same thing in both circumstances. In library evolution mode it implies a fixed layout, which wouldn’t be the case in non-evolution mode. Relatedly, it seems to me that one could still reorder properties of an @frozen noncopyable type in non-evolution mode while maintaining source compatibility.

We can of course say that reordering of stored properties remains banned for frozen types when not in evolution mode as a formal matter, but we won’t have a reliable mechanism for diagnosing issues, and in practice I worry that there will be an unofficial understanding that reordering stored properties is fine as far as source compatibility goes.

These subtle differences might not worry me so much if it weren’t so critical for users to understand exactly what they’re promising by making a type frozen.

ETA: if we were going to introduce something new I think we should consider something that is more explicit about the promise we’re making: that a stored property will never become a computed property and vice versa (and also, I suppose, that we will never add a new stored property?). We could annotate the type with @storedProperties(a, b) to indicate exactly the storage we expect to use. This has the added benefit of being difficult to accidentally break by failing to realize that a type is frozen—you’d need to change the declaration itself as well as the explicit promise about that property. If we want to be clear that order is not important here, this could be something like @storedPropertySet.

10

drain is not a consuming func, so it can’t use discard self.

2 Likes
11

Thinking through this a little bit more, I believe it’s necessary to support partial consumption to support types that have more than one non-copyable stored property:

struct FileDescriptor : ~Copyable { /* ... */ }
struct InlineBuffer<T> : ~Copyable { /* ... */ }
struct InputStream<T> : ~Copyable {
  var fd: FileDescriptor
  var inputBuffer: InlineBuffer<T>
  init(path: String) {
    inputBuffer = InlineBuffer<T>()
    fd = FileDescriptor.open(path: path)
  }
  mutating func read() -> InlineBuffer<T> {
    // We want to replace `inputBuffer` with an empty buffer, but leave `fd` alone.
    // Since both are move-only types, we can’t create a temporary copy of either one.
    // We _must_ partially de-initialize self, even if we wind up fully de-initializing it.
    let oldBuffer = inputBuffer
    let oldFd = fd
    self = InputStream(inputBuffer: InlineBuffer<T>(), fd: oldFd)
    return oldBuffer
  }
  deinit {
    fd.close()
  }
}

As @kavon pointed out, there is also the question of how to avoid the unwanted invocation of deinit.

If we are within a mutating (not consuming) method of a non-copyable type, SE-0390 says we cannot consume self without replacing it. SE-0390 also says that 1. re-assignment to a mutable binding is a consuming operation, and 2. if a method consumes a binding, then responsibility for executing deinit is deferred to the caller.

Taken together, these imply that deinit cannot occur during a mutating function. But does it make sense for the caller to deinit the callee that replaced its own value?

struct Queue<T> : ~Copyable {
  var buf: Buffer<T>
  mutating func mutateByCopyingBuf() -> Buf<T> {
    let oldBuf = copy buf
    self = Queue(buf: Buffer<T>())
    return oldBuf
  }
  mutating func mutateByConsumingBuf() -> Buf<T> {
    let oldBuf = consume buf
    buf = Buffer<T>()
    return oldBuf
  }
  deinit {
    buf.deallocate()
  }
}

func useQueue<T>(queue: borrow Queue<T>) {
  var buf = queue.mutateByCopyingBuf()
  // per SE-0390, `deinit` should run here
  // but that means buf will be deallocated before we can even use it!

  buf = queue.mutateByConsumingBuf()
  // if `deinit` runs above, should it also run here for consistency?
}

So let’s say we made it possible to discard self within a mutating method. That works when we re-assign self:

struct Queue<T> : ~Copyable {
  var buf: Buffer<T>
  mutating func mutateByCopyingBuf() -> Buf<T> {
    let oldBuf = buf
    discard self
    self = Queue(buf: Buffer<T>())
    return oldBuf
  }
  deinit {
    buf.deallocate()
  }
}

func useQueue<T>(queue: borrow Queue<T>) {
  var buf = queue.mutateByCopyingBuf()
  print(buf) // no longer crashes!
}

…but it doesn’t seem applicable when only consuming one of self’s stored properties:

extension Queue {
  mutating func mutateByConsumingBuf() -> Buf<T> {
    let oldBuf = consume buf
    discard self // What???
    buf = Buffer<T>()
    return oldBuf
  }
}

Another approach, if swap(_:_:) is able to exchange two move-only values, would be to avoid the partial de-initialization altogether:

struct Queue<T> : ~Copyable {
  var buf: Buffer<T>
  mutating func mutateByConsumingBuf() -> Buf<T> {
    let oldBuf: Buffer<T> = Buffer<T>()
    swap(&buf, &oldBuf)
    return oldBuf
  }
  deinit {
    buf.deallocate()
  }
}

Of course, this is kind of cheating.

12

You are correct. To do so, you would need to define a consuming method to get out the buffer and use discard. But you would still have the same sort of issue.

Keeping in mind that I was typing quickly and that was the source of that error... I am going to add to the original text above with an edit a real compilable and testable implementation of a FixedSizeQueue that we can use to guide our discussion. I think it will help avoid small mistakes like that in our discussion since any one of us can just compile the example while playing with it to discover small mistakes like that.

I actually had some text to add about discard but I forgot to add it to the original document. I will add it above and then ping the thread. Let's discuss this after that. Thank you for pointing this out.

Just to go over for anyone listening into this thread who may be unfamiliar with the situation Kavon is talking about, if one throws from a throwing init or one returns nil from a failable deinit, we clean up the rest of the type just as we would but we do not emit the deinit. For anyone who is interested in validating this, here is a small example:

class C {
    var k1: Klass 
    var k2: Klass

    init?() {
        k1 = Klass()

        if !bool {
            return nil
        }

        k2 = Klass()
    }
}

the -Onone sil we emit along the failure path is:

bb1:                                              // Preds: bb0
  %15 = ref_element_addr %0 : $C, #C.k1           // user: %16
  %16 = begin_access [deinit] [static] %15 : $*Klass // users: %18, %17
  destroy_addr %16 : $*Klass                      // id: %17
  end_access %16 : $*Klass                        // id: %18
  %19 = metatype $@thick C.Type                   // user: %20
  dealloc_partial_ref %0 : $C, %19 : $@thick C.Type // id: %20
  %21 = enum $Optional<C>, #Optional.none!enumelt // user: %22
  br bb3(%21 : $Optional<C>)                      // id: %22

which as one can see, involves us destroying just k1 and not k2 since it was not yet initialized. Then dealloc_partial_ref is invoked which just cleans up the memory without calling the deinit.

With that in hand, if I am understanding you correctly Kavon, I think what you are asking is can we make it so that we destroy the rest of the live parts of the type and not invoke the deinit like we do in the class example above.

While those semantics are implementable in the compiler, I think that it creates the wrong default behavior. Specifically off the top of my head:

  1. In the cases above, an init has not completely run so it is reasonable to say that a user cannot expect any type level contract/invariant like running a deinit to be enforced by the language. Once an object has been completely initialized our programming model should err on the side of the deinit running if it is provided due to that type level contract. That is an important invariant that users would expect by default to have unless there is something very explicit in the code like discard that signifies the break from the default. In contrast by making an assignment just cause the deinit to be turned off we have inserted into our programming model a very easy mistake to make that is also very hard to diagnose. Consider for instance how many assignment statements are in a large app and imagine a programmer having to track down which one of those specific assignment operations caused a deinit to be turned off. That is what is so great about discard! It makes turning off the deinit explicit in the code in an unmistakeable way that is easy to audit and understand. In the situation I described above, the programmer would just need to search through the program for all discard statements to determine what points the bad behavior occurred in.

  2. By doing this, we are assuming that whether or not a deinit should run is /always/ tightly coupled with the validity of a stored property on the type. Consider a simple situation where one has a deinit that calls a logging API when a type is destroyed. The deinit itself does not use any of the stored properties and just calls a logging API directly. I think it would be surprising to users that consuming a property of such a type would cause the deinit to not run.

13

As I mentioned in my response above, this is actually not an issue. You can just create a separate consuming func that does the discard. See this Interpreter test I landed that provides a simple fixed size queue example (see bottom of post). That being said, I do think that from an expressivity perspective it may make sense to add discard support to mutating functions and I am not against that since one would be able to just get rid of writing an extra consuming function.

1 Like
14

See my response above about using a consuming helper function and take a look at my FixedSizeQueue implementation I posted (... but plz ignore that in that implementation I just return the entire memory rather than a slice or something of the like)

15

Seems like the easy solution for this is what you mention: require a discard self if self is in a partially consumed state and you're exiting the method. If you do want the deinit to run, you have to assign value(s) back into all of the consumed fields.

Deinits implicitly "use" all of the stored properties to destroy them, so that's something people should know. It's just that some kinds of stored properties have no-op destruction.

16

I think what you are suggesting is that rather than just ban all partial consumption with deinits like we currently do in tree, we instead add a diagnostic that in such a situation tells the user that they need to use discard or reinitialize the type. Did I understand correctly, Kavon?

I would be fine with adding such a diagnostic... but I think adding it would naturally be an extension of this work. We can safely loosen the implementation later to support such a diagnostic while preserving source stability since we are expanding the set of valid programs.

While I think that may be obvious to compiler engineers, I do not think it would be obvious to a random user. It seems like something that a user would hit and then would have to go on stack overflow.

3 Likes
17

Yes. Since I think banning partial consumption entirely if there is a deinit is very limiting and not necessary: if there is no deinit but we permit partial consumption without reinitialization, we'd need to do the same destruction of the remaining fields. The only difference is that no indicator like a discard is required to signal that the deinit isn't run, since there is none.

Thus, I think if we figure out a way to support partial consumption without reinitialization, then types with deinits won't be an issue. Is partial consumption without reinitialization something you're considering here?

When self is partially initialized but not put back together and a diagnostic tells users they need to write discard self, that means the deinit isn't going to run. It's safe to assume people making use of these features understand what that statement means.

If someone is curious why they can't let the deinit run, then yeah they can look to the proposal for the reasoning, etc. I didn't mean to suggest the reason why had to be obvious to users. They only need to know that the deinit isn't going to run in that case.

18

Normally I think that language features should be as accessible as possible, however in the case of non-copyable types, we can probably assume that users of the feature are more advanced, since it is a rather advanced and niche feature of the language. Therefore I find it totally reasonable that users of non-copyable types understand roughly how deinits work.

19

Is that true, though? I thought non-copyable types were pitched as the way to generally do RAII in an efficient, concurrency-safe way (as opposed to using classes)…? If so, won't they pop up all over the place?

I'm not sure whether by "user" you mean the person writing the non-copyable type or the person using it, but either way I get the feeling these will be things that lots of people ultimately use.

1 Like