Modify Accessors

Evolution Pitches
268

Reviving this thread because I just noticed this part of the proposal.

This is inconsistent with the stated rationale for try, which was that it is important to mark all the places that can throw with try.*

In fact, I now realize I've written a number of accessors that are broken in the face of a throwing RHS context due to the behavior proposed. You might say the whole point of modify (and especially read) is that the author gets to inject some cleanup code after the yield. But as an author, nothing forces you to confront the fact that the code after yield might be skipped, so it's really easy to overlook. Given that:

  • error-handling paths are rarely tested in practice, and
  • every other function an error can propagate from has to be explicitly labeled [re]throws, so you get used to being protected from this kind of mistake and will be unwary,

:foot: :gun: ouch :frowning_face:

I came to this post by way of wondering, “hmm, I wonder what happens when an error propagates through a yield?” and before I found this part of the pitch, I did some experimentation. My first instinct was that the code after yield would be run unconditionally, since it's invariably going to be cleanup code, but turns out I was wrong. “Okay, somebody probably figured you might want to do something different when there's no error,” I thought. So I tried putting a do { … } catch { … } around the yield. But the compiler doesn't accept that.

Eventually I hit on using defer, which obviously has to work… but if you did want different behavior when an error propagates, forces you into this:

modify {
  var errorOccurred = true // I have to lie, at least temporarily
  defer { 
    if errorOccurred {
      doSomething()
    }
    else {
      doSomethignDifferent()
    }
  }
  yield &whatever
  errorOccurred = false
}

This is clearly awful. We have a standard way to get different behavior depending on whether an error propagates: it's called do { … } catch { … }. I shouldn't have to use a completely different, contorted idiom to do the same thing just because I'm writing an accessor. What we've created is a context in which the Swift “rules of the road” for error handling are suddenly, and subtly, very different.

(Sorry for not reading the rest of this long thread before posting this suggestion; others have better ideas)

The most surgical fix I can think of for this problem would be to say that if the accessor contains non-defer-enclosed code that can execute after yield, the yield must be marked with try. The diagnostic could be something like, “statement will not be reached when an error is thrown from yield. Please mark the yield with try or move the logic into a defer block before the yield to have it execute unconditionally” (plus fixits, of course).

* I happen to know that it isn't important, and in fact only a very few kinds of places can benefit from such marking. I don't think it's too late to mitigate that design mistake, but that's a topic for a different thread.

5 Likes