Throwable accessors

I would not worry about rethrows. It's useful with subscripts that take closures, especially autoclosures, but it is definitely not a minimum requirement.

2 Likes

Thanks @John_McCall, I have got the type checking working now for subscripts :slight_smile: I am using a custom ASTVisitor approach and it works fine. I have a PR ready that reverts the commit you mentioned. Do you think it's worth reverting or should I stick to my approach?

Are you actually handling inout arguments and potentially-mutating base accesses correctly? It’s certainly possible to do these things with a visitor, but my concern is that you’re going to be “approximately correct”.

@John_McCall I am unsure, could you take a look at my branch? https://github.com/theblixguy/swift/compare/master...theblixguy:feature/throwable-accessors I have added some tests as well.

1 Like

@anandabits I've fixed it now, so if you declare a protocol with a non-throwing accessor (requirement) and the conforming type (witness) declares it as throwing then you get a diagnostic (as expected).

Awesome!

1 Like

Can you create a draft PR against upstream main branch so the code reviews can take place there?

1 Like

Done!

@John_McCall I have created a WIP draft pull request for code review: https://github.com/apple/swift/pull/22749

3 Likes

@John_McCall Have you had a chance to look at the PR? I would really like to finish the implementation! :slight_smile: The type checking bits are nearly done - I suppose the next step would be to support throwing of _read and _modify in SIL.

Sorry, I haven't; the last couple of weeks have been really busy. I'll go take a look now.

2 Likes

How would key-paths work on throwable accessors?

struct S {
  var a: Int { get throws { return 42 } }

  var b: Int { return 42 }
}

I cannot solve this problem in my head without new key-path classes and typed throws where when throws is omitted it equals throws(Never).

func extract<Root, Value, Path, Error>(
  _ path: Path,
  from root: Root 
) throws(Error) -> Value
  where 
  Path: KeyPath<Root, Value>,
  Error: Swift.Error
{
  return try root[keyPath: path]
}

Usage:

let s = S()

_ = try! extract(\.a, from: s) // extract<S, Int, ??, Swift.Error>
_ = extract(\.b, from: s) // extract<S, Int, KeyPath<S, Int>, Never>

But new key-path classes would require new OS :cry: even though the other features are potentially backwards compatible.

cc @Joe_Groff

Can we potentially retroactively introduce a generic Error type parameter to KeyPath<..., Error> and it's subclasses?

Any thoughts regarding this issue @Joe_Groff @John_McCall?

I don’t know. KeyPaths are already limited in what combinations of mutating-ness are supported — notably, you can’t have a mutating get — so it wouldn’t be unprecedented for them to simply not support throwing accessors, either.

Actually in a conversation on Slack @beccadax had an idea that for backwards compatibility the compiler could box these into Result. So maybe we can say that \S.a would produce a KeyPath<S, Result<Int, Error>>? But that would limit the way we can express the above extract method generically. It would need to return Result all the time, which is fine I guess.

I also noted that boxing the result in Result would only work for reading, not writing. It's probably possible, but I'm not sure it would be worth the effort; throwing accessors are a big change and I don't think it'd be terrible to ask people to use a new runtime if they want to use them with key paths.

1 Like

You need a new runtime to use a throwing key path anyway. The existing entrypoints aren’t going to propagate errors out.

1 Like

It’s a shame that this appears to have lost steam. Are you still interested in pursuing it, @suyashsrijan ?

Yeah, I’m still interested in implementing this (and writing a proposal which I think would be required). I believe there are three things to do:

  1. Restore LValueAccessKind in the AST/Sema. The computation for that used to exist there but it was moved into SILGen because it wasn’t fully correct. We need to move it back into AST/Sema. This will also help fix some other bugs, like SR-11106, SR-10123, and SR-9960.
  2. Use LValueAccessKind and teach the type checker about throwable accessors (it already knows about throwable functions, so this is the easy part). My WIP PR was focused on this, but it used the same complicated logic that we use for checking throwing functions. @John_McCall suggested it would be much better if we use LValueAccessKind instead.
  3. Teach SILGen about throwing coroutines (_read and _modify). I don’t know much about this part, but we can discuss it later.

I think the first step would be to do (1), but I think I need to think about the best way to implement it in the AST/Sema (maybe I can use a visitor?). Also, I need to gain a better understanding of the LValueAccessKind computation (I haven’t looked at the SILGen code) and we need to make sure the computation is correct. cc @Slava_Pestov who might have some thoughts about how to move forward with LValueAccessKind change.

2 Likes

The old computation worked by storing the LValueAccessKind inside every Expr, and updating it when Exprs were built while applying the solution. I think it would be cleaner to separate it out into a visitor that can be applied to Exprs that appear in LValue position to compute the access kind as needed. Such a visitor could also be invoked from various places where diagnostics are emitted.

Was this proposal thrown by the wayside when the Effectful Read-only Properties Proposal SE-0310 was added to Swift 5.5, or are we still going to get throwing setters, too?

1 Like