Modify Accessors

That makes good sense. I guess the main reason I am still entertaining @orobio's idea is that relying on defer forces one to write steps out-of-order, but I'll withhold judgement on that until I've updated my accessors to use defer.

I've been looking at it more as being a property of the coroutine state. The coroutine is either active or it needs to terminate due to that the caller no longer needs it. Using a thread-local property for this might work though.

You're talking about error handling here, so I'd like to stress once more that there are many reasons that could cause the coroutine to terminate; the caller breaking out of a loop, returning, etc. A thrown error is just one of the possibilities, and I'm assuming that the coroutine in almost all cases does not have to deal with the reason it terminates (my assumptions might be wrong though).

I have no strong opinion on this, but it seems reasonable.

It'd be great to get some more input from real world usage. I really dislike the out-of-order nature of the defer solution, but perhaps in practice it's not as bad as it seems.

This is a feature that can be designed in many different ways. It would be great if the most common use cases can be written naturally, but what the most common use cases are might still be a bit unclear (at least to me).

I mean, you could say this, but that doesn’t mean it will actually fit users’ existing mental models. For five years now, try has indicated something that you can catch. Suddenly having an uncatchable try is arguably more startling than potentially not resuming after a yield. After all, yield is a novel keyword that could mean anything; users think they already know what try means.

I continue to believe the best solution is for yield to give a Bool result which, like most result values in Swift, the compiler will warn you not to implicitly ignore. We could loosen this warning if yield is the final statement in the body to avoid forcing silly _ = constructs. This design might require a tiny bit more code at the yield site, but it’s the only solution I’ve seen discussed here that users will understand how to use correctly right away without needing to be taught.

6 Likes

Since that control flow is so problematic, could we just get rid of the need to bother with it? Say you have this generator:

func generate() {
   yield a
   yield b
   yield c
}

and the first yield fails, we could just continue running the function as if nothing happened. The following yields would just become no-ops. As far as the generator is concerned, yield can't fail, it'll just do nothing if the yielding context disappears.

We should also have a way for the generator to avoid doing useless work, but as an optimisation. Just check for the context being broken:

func generate() {
   yield &a
   if brokenYield { return }
   yield &b
   if brokenYield { return }
   yield &c
}

So if you don't bother handling the error path, your generator will continue to behave correctly. The only downside is it'll do more work than necessary.

There's also no need to check after every yield: only check when it actually avoids work:

func generate() {
   var (a, b) = fetchAB()
   yield &a
   yield &b
   commitAB(a, b)

   if brokenYield { return } // this is a good point to stop

   var (c, d) = fetchCD()
   yield &c
   yield &d
   commitCD(c, d)
}

That's still implicit control flow, only it's much subtler. Just one example of subtlety: if there are side-effects as part of the yield expression, do they execute? Doesn't matter what your answer is; the point is you have to think about it.

Not that it matters much for the discussion at hand, but that was my point: if you invert the flag, it’s not a lie: it haven’t yet yielded successfully.

But I digress. ¯_(ツ)_/¯

2 Likes

There's not question in my mind that it should behave the same as throwing function calls with inout parameters: setters are run when an exception is thrown in such a case; so setters must be run when yield fails too. I think it'd be problematic if yield behaved any differently.

To be clear, all the code you see in the generator would run as written until the end of the function. If you have yield &a it'll run the getter and setter of &a (or _modify) even if there's no one to yield to anymore, just as if you were passing it inout to a function with an empty body. There is really no error path in the generator (unless you write one explicitly), it just runs it course.

While maybe not optimal, the generator should behave correctly even if you don't give a single though about errors, and that's the point.


The last example of my last post illustrates how good a non-failing yield is at expressing the control flow. Here's the equivalent rewrite using defer assuming a yield that exits the generator on failure:

func generate() {
   do {
      var (a, b) = fetchAB()
      defer { commitAB(a, b) }
      yield &a
      yield &b
   }
   do {
      var (c, d) = fetchCD()
      defer { commitCD(c, d) }
      yield &c
      yield &d
   }
}

The problem is it's likely you'll forget to put the commit calls in a defer with a do block for correctly scoping those defer. I said "likely you'll forget", but I should have said you'll definitely forget it you aren't mindful of the error path while writing the code.

Using finally makes the flow a bit more linear, but doesn't really help to remind you you need to write all this control flow in the first place:

func generate() {
   do {
      var (a, b) = fetchAB()
      yield &a
      yield &b
   } finally {
      commitAB(a, b)
   }
   do {
      var (c, d) = fetchCD()
      yield &c
      yield &d
   } finally {
      commitCD(c, d)
   }
}

I didn't want to write an example using yield … else because it's just awful having to duplicate those commit calls. That said, the obligatory else would be a good reminder of the error path. People generally want to focus on what they want the code to do when writing code, not error handling and they might see the compiler complaining as an annoyance you need to silence by writing an empty else. If you do that you end up with the same problem as above, but at least you were warned I guess.

I think it's far better if we can just get rid of the error paths when they're not needed.

3 Likes

Drive-by comment: we should allow nonmutating modify accessors, for use in cases where the type implements reference semantics.

One example of such a case is ManagedBufferPointer.header, which unfortunately has a mutating _modify in Swift 5.3. :frowning:

7 Likes

I agree with John. Maybe if yield is the last line of modify, try can be omitted. But in other cases swift could require try and that would be the "tell" that the code after this may not run and to be careful.

1 Like

i wonder if swift could have used the same approach as C++ - their std::vector / std::map are also COW-able value types and have to deal with this issue.

As this thread approaches its 2nd birthday :partying_face::birthday:, a quick summary of where thing stand:

  • (ABI stable) The standard library makes extensive use of _modify
  • (ABI stable) Foundation's new AttributedString makes use of _modify
  • (ABI stable) swift-system's FilePath.ComponentView makes use of _modify
  • swift-collections makes use of _modify (just one example, there are many)
  • swift-nio tries to do the right thing and has a modify method instead :pleading_face:. Bless 'em.

In my own projects, I of course avoid underscored functions or attributes, because they technically are not stable and could disappear or change semantics without notice. The only exception is _modify. Unlike most underscored features, using _modify accessors in third-party code is well within the realm of "acceptable risk" IMHO.

It's critical to many API designs, and used so extensively (by ABI stable libraries, no less), that the chance of it being removed or suddenly becoming incompatible with current behaviour is basically zero.

26 Likes

It saddens me that the modify accessor as is written has become de facto standard long before it has entered a wider-audience discussion. More so now that people start using it extensively. From the design perspective, that only makes it much harder to iron out the wrinkle around the feature, and this accessor sure has a few.

Don't get me wrong, from the practical standpoint, the modify accessor is an important and a much-needed feature for essentially any performance-sensitive code, especially in a CoW-dominant language like Swift. It's just that it could have been much more. It could have felt like a deeply-integrated feature rather than a tacked-on one, could have.

9 Likes

Some of the ideas in this thread, like requiring a try yield with the ability to catch errors, would (I'm guessing) probably not be ABI-compatible with libraries that have already shipped. And some libraries, like swift-system and swift-collections, are also shipped in source form, so any projects with a fixed-version dependency would fail to build if the compiler stopped accepting _modify with the exact behaviour it currently has.

So yeah, (underscored) _modify is de facto stable and the compiler will have to support it for a long time. But that doesn't mean we can't change things for the separate (non-underscored) modify feature.

I don't think that there's anything from a policy perspective that would prevent special handling being added to allow _modify to be updated in ways that wouldn't break Apple frameworks but may be incompatible with arbitrary libraries which depend on it. It is, after all, an underscored feature which has not been accepted via the evolution process.

It's also possible that there are non-ABI implementation details of _modify that are not relied on by Apple frameworks, but are relied on by third-party code. As a policy matter, again, we should not consider the behavior of _modify constrained by these clients.

Now, it may be that, pragmatically, such special handling wouldn't be worth the effort or wouldn't be possible to achieve in practice. Just want to make sure we don't overstate the stability of _modify. Any changes could be made without an evolution proposal, only taking Apple's migration story into account.

1 Like

I don't think that is as sure as you think it is, and I certainly would discourage anyone from taking this as advice that one can just use _modify and expect no consequences.

The only thing that is ensured by its use in ABI-stable libraries is there will have to be some (probably private) feature to let the libraries that use it right now continue to expose the same ABI. The ABI and spelling of any modify that is eventually accepted need have nothing in common, and _modify itself could disappear or completely change behaviour at any point.

1 Like

Try to imagine what would happen if _modify actually did change its behaviour - it would break the builds of everybody using source libraries such as swift-system and swift-collections. At the very least, there would need to be a lengthy transition period where both implementations coexisted, until it was certain that nobody was using an older version of the package.

The ABI stability issue is one thing. The source stability requirements for these packages is harder to get away from.

Only if it changes behavior it a way that breaks those specific packages. I don't know that it would break the source stability promise to update the behavior for, say, all modules not named Swift, SystemPackage, Collections, etc., or to update behavior that Apple packages happen not to depend on but third-party libraries might. The compatibility promise for an underscored, unofficial feature is quite narrow, absent any official guidance from the core team/Apple themselves.

ETA: source stability could be achieved even by adding code which specifically checked for an exact source match (down to the character) of each existing use of _modify in Apple frameworks and conditioning its new behavior (or removal) based on that. I don't think that's a particularly likely future, but it wouldn't violate existing ABI or source compatibility promises to update the language in such a way.

Even that wouldn't be sufficient; module names are not universally unique. So this hack would need to permeate to the package manager... and even that wouldn't be sufficient; you might be using a mirror or fork.

What happens if they need to fix a bug? It would break _modify on older compilers which don't recognise the new source.

But yeah, of course it is not an official language feature and generally I don't support using those either; still, in this specific case, I think it stands that:

Not completely zero, but unlikely enough that I wouldn't worry about it. I'm more worried about random, accidental regressions than _modify suddenly and intentionally changing.

1 Like

Yeah, I think we basically agree. I just want to make sure that we're precise in this thread about what "behavior" and "regression" encompasses. To the extent that Apple's libraries don't depend on a certain behavior or feature, I don't think it really makes sense to even call a change a "regression," even if it breaks third-party code.

1 Like

There are no source stability commitments when it comes to underscored keywords or attributes and no-one should have any expectations in this regard, either at compile time or runtime.

However, if _modify were to become modify, this would also provide an opportunity to mitigate source breakage by only applying the new syntax or semantics to modify and not _modify, if that proved a worthwhile expenditure of effort.

11 Likes