Entry 8: Incomplete Computed Properties

It's been a long time since I managed to land a change to the Swift compiler. (Has it really been almost 2 years?!) Being a dad, changing jobs, other projects, and the difficulty (for me) of working in C++ have all keep me away (although I have tried on a few occasions). But I'm glad to have finally made another contribution.

When I'm working in Swift, I try to make note of confusing errors that I or others run into. When I have time to work on the Swift compiler, these are the things that I try to improve.

Lately, I've been bothered by this one:

struct S {}

extension S {
    var foo: Int {
    }
}

I've written a computed property in an extension, but haven't filled out the implementation. I do this frequently. I decide that I need a property, write its declaration, and then pause to plan my implementation.

But at this point, Swift gives me 2 errors:

example.swift:5:5: error: computed property must have accessors specified
    }
    ^
example.swift:4:9: error: extensions must not contain stored properties
    var foo: Int {
        ^

And not only that, but:

1. They appear at separate points in the declaration
2. One says the property is computed and the other says that it's stored

So I decided I'd try to improve this by removing the 2nd error.

My first step was to find the source of the errors. This is pretty easy, since you can work backwards from the error message:

1. Search the project for a string from the error, recognizing that parts of the message can be variable.
2. Once you've found the error string in a .def file, search for the name of the diagnostic.
3. Set breakpoints where the diagnostic is emitted.
4. Run the code again to see where exactly the diagnostics are emitted in this case.

In this case, the computed property error comes from the parser and the store property error comes from the typechecker. For the latter error, the typechecker emits the error if (1) the property is an an extension and (2) it has storage—i.e. exactly what the error says.

To prevent this, I decided to alter the parser to treat an empty set of braces as a getter—instead of complaining about the getter and marking it as a setter. I copied-and-pasted some code to add a getter and verified that it would fix the issue.

Then I had to make sure that I didn’t break any other diagnostics. In particular, this protocol diagnostic depends on the existing storage behavior:

protocol ProtocolGetSet2 {
  var a: Int {} // expected-error {{property in protocol must have explicit { get } or { get set } specifier}} {{14-16={ get <#set#> \}}}
}

But there were also some other cases of computed property must have accessors specified that I didn’t want to break. I ran the test suite to find some examples, then decided to add the getter only when in an extension:

     // In an extension, add a getter to prevent an "extensions must not contain
     // stored properties" error.
     if (Flags.contains(PD_InExtension)) {
       auto getter = createAccessorFunc(
           Tok.getLoc(), /*ValueNamePattern*/ nullptr, GenericParams, Indices,
           StaticLoc, Flags, AccessorKind::Get, storage, this,
           /*AccessorKeywordLoc*/ SourceLoc());
       accessors.add(getter);
     }

At this point, I ran the tests and opened a PR.

On the PR, I received feedback that it’d be best to tackle this a different way: instead of adding a fake getter in the Parser, adjust the storage request (StorageImplInfoRequest::evaluate) to return that there’s a getter.

So I reverted my fix, changed the default in this method to return Get, and re-ran my tests. It did fix the 2nd diagnostic, but it also broke other tests (as expected), so this was a workable solution.

I added another if to this method to check whether (1) this was in an extension and (2) had braces, and returned Get in this case.

   // Extensions can't have stored properties. If there are braces, assume
   // this is an incomplete computed property. This avoids an "extensions
   // must not contain stored properties" error later on.
   } else if (isa<ExtensionDecl>(storage->getDeclContext()) &&
              storage->getBracesRange().isValid()) {
     readImpl = ReadImplKind::Get;

And that worked!

As usual, this involved a lot of fumbling around. But by choosing something small, persevering, and receiving PR feedback, I was able to contribute a fix for a diagnostic issue that plagues me almost daily. This was humbling, educational, and an investment in my day-to-day happiness.

The PR is here.