Throwable accessors

Still needs a bit more work for type checking subscripts correctly, which I am currently working on (this one is a bit tricky, but I'm close). What else needs to be done? Everything else seems to be working fine (compiles, runs, throws the error as expected) according to my tests, but unsure if I am missing some scenarios. @John_McCall @Slava_Pestov

Just to give an example of what works so far:

  1. Computed properties with throwing getters and setters
  2. Subscripts with throwing getter and setter

Example:

struct Foo {
  private var _array: [Int] = [0, 1, 2]
  
  enum FooError: Error { case accessor }
  
  subscript(x: Int) -> Int {
    get throws {
      if _array.indices.contains(x) {
        return _array[x]
      } else {
        throw FooError.accessor
      }
    }
    set {
      _array[x] = newValue
    }
  }
  
  var array: [Int] {
    get throws {
      if !_array.isEmpty {
        return _array
      } else {
        throw FooError.accessor
      }
    }
    set throws {
      if newValue.isEmpty {
        throw FooError.accessor
      } else {
        _array = newValue
      }
    }
  }
}

var instance = Foo()
let _ = try instance[1] // 1
let _ = try instance[3] // FooError.accessor
let _ = try? instance[3] // Ok
instance[0] = 3 // Ok

var anotherInstance = Foo()
let _ = try anotherInstance.array // [0, 1, 2]
try anotherInstance.array = [] // FooError.accessor
try anotherInstance.array = [1, 2, 3] // Ok
try? anotherInstance.array = [] // Ok

Both mutating and non-mutating variants work.

4 Likes

The main thing you need to do for type-checking is to resurrect an old piece of code we had that stored an AccessKind on l-value expressions. It was removed here. That should make it straightforward to figure out what accessors are being used.

That's probably something you should go ahead and prepare a PR for instead of leaving on your feature branch.

1 Like

Can you share a link to the WIP PR please? Since I haven't seen protocol related examples in this thread, I would like to know if in your tests you already created them.

protocol P {
  subscript(a: Int) -> Int { get throws }
  subscript(b: Int) -> Int { get throws set }
  subscript(c: Int) -> Int { get set throws }
  var a: Int { get throws }
  var b: Int { get throws set }
  var c: Int { get set throws }
}

Can a subscript and therefore get/set actually rethrow if it's argument was a throwing function?

Example:

struct S {
  subscript(x: () throws -> Int) -> String {
    get rethrows {
      return "\(try x())"
    }
  }
}

@John_McCall would it be a requirement for this patch to make internal _read and _modify (or how they were called) also throwable or can this be easily a follow-up patch?

Not trying to make anything harder for @suyashsrijan, just asking for clarification, since I'm tracking this patch.

The feature is not in any way complete without _read and _modify support. We automatically create _modify accessors for non-final storage in classes as well as storage that satisfies a mutable protocol requirement.

1 Like

Protocols with throwing accessors work although there's an issue where a throwing getter can fulfil the requirement of a non-throwing getter (as defined in protocol). rethrows doesn't get parsed, but unsure if it's something we want to add right now.

This sounds pretty bad. It will be fixed before this lands on master (assuming it is accepted), right?

Yes, at the moment, I want to wrap up the type checking bit before moving on to other areas.

Great!

1 Like

In the sense of this post (which is not available in Swift right now and is not even decided yet) I would expect that any get/set (read/modify) throws all the time. If omitted then it throws Never and therefore works like in the current Swift version.

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.