Is there a warning about accidentally overriding a property with a method?

Using Swift
1

I see quite some code accidentally “override” a property of the superclass using a method, for example:

public class MyViewController: UIViewController {

    public func preferredContentSize() -> CGSize {
        // only for demo purpose
        return CGSize(width: 100, height: 100)
    }

}

Usually such code comes from migrating ObjC code:

@implementation MyViewController

- (CGSize)preferredContentSize {
    // only for demo purpose
    return CGSizeMake(100, 100);
}

@end

It will be great if the compiler could give a warning (could be opt-in) when a subclass defines a method with the same name as a property defined in superclass, as it is unlikly an intended coding practice.

1 Like
2

It works the other way too. WHY?!

class Super {
  func overloaded() { }
}

final class Sub: Super {
  var overloaded: Void { }
}

let sub = Sub()
let property: () = sub.overloaded
let method: () -> _ = sub.overloaded
3

The standard library makes use of this form of overloading, for example types conforming to Collection have both a property and a method named first and count.

4

That is dandy! But not the same. first takes a parameter. I.e. this compiles:

class C {
  var first: Void { }
  func first(_: some Any) { }
}

This does not:

class C {
  var first: Void { }
  func first() { }
}

But split the two things across the subclass boundary, and that compiles too, as we showed.

3 Likes
5

Good point. This works too though:

class C {
  func x() {}
}

class D: C {
  func x(_: Int) {}
}

Maybe it shouldn't? It's not so clear-cut though.

6

this behavior is intriguing, and i too am curious to know 'WHY?!' things behave this way... is this just an edge case that has not been considered (and has not clearly caused problems), or is it more intentional? do the vtables not support 'disambiguating' between methods & properties on the same base type or something (a cursory inspection of the SIL seemed like they are at least printed out differently, so maybe are distinguishable)?

as far as diagnosing it goes, that seems like something that'd be fairly straightforward to support as an opt-in diagnostic i'd think... 'just' walk up the inheritance hierarchy for certain overridden decls looking for the right sorts of patterns would perhaps do it (though it might be more complicated than i'm imagining, as things so often are)?

slightly tangential, but here is a further oddity inspired by this:

the shadowing apparently is allowed on the base class if the method & getter are async (which is seemingly not true for other effects, like throws)

class Super {
    func overloaded_async() async { }
    var overloaded_async: Void { get async {} } // ✅ two signatures happily co-existing
}

but if you override via a subclass and don't properly annotate things, you get a SIL verifier error (and maybe a miscompilation?):

final class Sub: Super {
    override var overloaded_async: Void { } // not `async`
}

SIL verification failed: vtable entry for #Super.overloaded_async!getter must be ABI-compatible
  sync/async mismatch
  @convention(method) (@guaranteed Sub) -> ()
  @convention(method) @async (@guaranteed Super) -> ()
Please submit a bug report (https://swift.org/contributing/#reporting-bugs) and include the crash backtrace.

reported that one here: SIL verification failure/miscompile when overriding async getter with non-async implementation · Issue #85332 · swiftlang/swift · GitHub

2 Likes
7

this has been lingering on my mind for the past couple weeks. i've played around a bit with adding logic to emit a diagnostic in situations like this – a basic proof of concept was fairly straightforward to add, but it's not entirely clear to me where to locate the checking nor the precise logic to use.

if we want to address this, two options immediately come to my mind – we attempt to ban support for this behavior entirely, or we just diagnose it as a warning (and that raises the separate question of whether such a diagnostic should be opt-in). the former would presumably require an evolution proposal, but in either case we need to pin down exactly the thing we're trying to point out as a potential mistake or cause of confusion.

my intuition is that the logic should be applied at some point when typechecking a declaration[1] and be something like:

  1. check that it's a property or zero-parameter method
  2. check that it's a member of a type with a superclass
  3. check that it's not an explicit override
  4. perform a lookup for the declaration's base name through the inheritance hierarchy
  5. if any ancestor contains a member that is a 'close enough' match, emit a diagnostic

exactly what criteria to apply in the final step is still not totally clear to me. maybe something like:

  1. one member is a property and the other a method
  2. the method is zero-parameter and has the same return type
  3. some consideration about effects matching or not?

we'll also need to pick wording to communicate to developers. i have not done much searching for prior art, so i'm not sure if there's a general term for this sort of thing already in use. a shadowing/override 'near miss' is what has been floating around in my mind.

at the very least, a bug or feature request in GH is probably warranted for this.

finally, can anyone think of arguments for why the current behavior is desirable (setting aside source compatibility)? is there a principled justification for allowing it when property/method name collisions on the same type are not?

  1. apply the logic to ValueDecls somewhere in TypeCheckDeclOverride? ↩︎

8

No, but the opposite "no" from what you're asking.

  1. Attempting to disallow it is too complicated to be worth implementing, and all previous attempts should be removed from the language.

E.g. These tests pass:

struct T: P {
  static var x: (Self) -> () -> Int { { _ in { 0 } } }
  func x() -> Int { 1 }
}

protocol P { }
extension P { var x: Int { 2 } }

let t = T()
var f = T.x // Can only refer to the static var. Remove the static var, and this is the instance method instead.
#expect(f(t)() == 0)
f = { $0.x } // Referring to the instance method requires making a new closure.
#expect(f(t)() == 1)

#expect(t.x() == 1)
#expect(t.x == 2)

The rules about what can and cannot have the same name are so confusing as to not be valuable.

  1. Swift precludes using verbs and their performative denominal forms in the same type, even when their signatures don't really match in a meaningful way. This leads to having to ruin APIs just to satisfy unknowable partially complete compiler rules.
enum Paint { }

struct Painter {
  func paint() { }
  var paint: Paint { } // Invalid redeclaration of 'paint'
}
3 Likes