Compiler should issue a warning when a subclass implementation with default values matches a parent implementation without them


(Frank O'Dwyer) #1

I think there is a clear case for a warning here, in that the introduction of a default parameter value in such cases is in effect an empty promise and just introduces unreachable code. Indeed 'unreachable code' should perhaps be the warning.

This is because as soon as there is a foo() method, whether by inheritance or a definition in the same class, it renders it impossible to invoke foo(x:) with its default value in the normal way, or in any interesting way. In fact it makes no difference at all if the default is 0, 3, or 5, or not written in the first place. You now have to be explicit. Therefore, adding the default seems to provide no utility at all and can only lead to the confusion the OP is talking about, and bugs. Furthermore it seems like a very loud indication that the programmer expected something else to happen and it violates the 'principle of least surprise' for anyone else reading the code.

Given all that, it seems like the syntactic vinegar of having to write a pragma to get a clean compile would be appropriate. It alerts other readers of the code that 'here be dragons', and it tells the code author that they should probably achieve what they intend in another way. Most likely, by renaming foo(), or removing the useless default for foo(x:).

Cheers,
Frank

···

On 5 Jan 2017, at 07:58, Rien <Rien@Balancingrock.nl> wrote:

As you know. there is no ambiguity, no warnings needed.
(The parameter is part of the identifier of the function)

Imo, this request falls into the category “do as I think, not as I say”.

That is a discussion without end. Personally I am against ANY warnings of this kind. The reason is that I want my code to compile warnings free (default compiler behaviour) and I do not want an extra pragma in the code to instruct the compiler that when I am calling “foo()” I do indeed want to call “foo()”.

Regards,
Rien

Site: http://balancingrock.nl
Blog: http://swiftrien.blogspot.com
Github: http://github.com/Swiftrien
Project: http://swiftfire.nl

On 05 Jan 2017, at 03:29, Wagner Truppel via swift-users <swift-users@swift.org> wrote:

Hello,

I wasn’t sure whether to post this message here, at swift-dev, or at swift-evolution. so I’ll try here first. Hopefully it will get to the right group of people or, if not, someone will point me to the right mailing list.

I came across a situation that boils down to this example:

class Parent {
  func foo() {
      print("Parent foo() called")
  }
}

class Child: Parent {
  func foo(x: Int = 0) {
      print("Child foo() called")
  }
}

let c = Child()
c.foo() // prints "Parent foo() called"

I understand why this behaves like so, namely, the subclass has a method foo(x:) but no direct implementation of foo() so the parent’s implementation is invoked rather than the child's. That’s all fine except that it is not very intuitive.

I would argue that the expectation is that the search for an implementation should start with the subclass (which is does) but should look at all possible restrictions of parent implementations, including the restriction due to default values.

At the very least, I think the compiler should emit a warning or possibly even an error.

Thanks for reading.
Cheers,

Wagner
_______________________________________________
swift-users mailing list
swift-users@swift.org
https://lists.swift.org/mailman/listinfo/swift-users


(Wagner Truppel) #2

Exactly.

I would only add that renaming might not be the correct solution in some cases as, then, instances of the subclass would still be able to invoke the superclass method and that might not always be acceptable.

Wagner

···

On 5 Jan 2017, at 11:09, Frank O'Dwyer <frankodwyer@netscape.net> wrote:

I think there is a clear case for a warning here, in that the introduction of a default parameter value in such cases is in effect an empty promise and just introduces unreachable code. Indeed 'unreachable code' should perhaps be the warning.

This is because as soon as there is a foo() method, whether by inheritance or a definition in the same class, it renders it impossible to invoke foo(x:) with its default value in the normal way, or in any interesting way. In fact it makes no difference at all if the default is 0, 3, or 5, or not written in the first place. You now have to be explicit. Therefore, adding the default seems to provide no utility at all and can only lead to the confusion the OP is talking about, and bugs. Furthermore it seems like a very loud indication that the programmer expected something else to happen and it violates the 'principle of least surprise' for anyone else reading the code.

Given all that, it seems like the syntactic vinegar of having to write a pragma to get a clean compile would be appropriate. It alerts other readers of the code that 'here be dragons', and it tells the code author that they should probably achieve what they intend in another way. Most likely, by renaming foo(), or removing the useless default for foo(x:).

Cheers,
Frank

On 5 Jan 2017, at 07:58, Rien <Rien@Balancingrock.nl> wrote:

As you know. there is no ambiguity, no warnings needed.
(The parameter is part of the identifier of the function)

Imo, this request falls into the category “do as I think, not as I say”.

That is a discussion without end. Personally I am against ANY warnings of this kind. The reason is that I want my code to compile warnings free (default compiler behaviour) and I do not want an extra pragma in the code to instruct the compiler that when I am calling “foo()” I do indeed want to call “foo()”.

Regards,
Rien

Site: http://balancingrock.nl
Blog: http://swiftrien.blogspot.com
Github: http://github.com/Swiftrien
Project: http://swiftfire.nl

On 05 Jan 2017, at 03:29, Wagner Truppel via swift-users <swift-users@swift.org> wrote:

Hello,

I wasn’t sure whether to post this message here, at swift-dev, or at swift-evolution. so I’ll try here first. Hopefully it will get to the right group of people or, if not, someone will point me to the right mailing list.

I came across a situation that boils down to this example:

class Parent {
func foo() {
     print("Parent foo() called")
}
}

class Child: Parent {
func foo(x: Int = 0) {
     print("Child foo() called")
}
}

let c = Child()
c.foo() // prints "Parent foo() called"

I understand why this behaves like so, namely, the subclass has a method foo(x:) but no direct implementation of foo() so the parent’s implementation is invoked rather than the child's. That’s all fine except that it is not very intuitive.

I would argue that the expectation is that the search for an implementation should start with the subclass (which is does) but should look at all possible restrictions of parent implementations, including the restriction due to default values.

At the very least, I think the compiler should emit a warning or possibly even an error.

Thanks for reading.
Cheers,

Wagner
_______________________________________________
swift-users mailing list
swift-users@swift.org
https://lists.swift.org/mailman/listinfo/swift-users


(Kenny Leung) #3

Hi All.

I think this is a case of ambiguity that the compiler is not catching at the call site. Consider this example:

private class DefaultValues {
    
    // If this method exists, it compiles and it called.
    // If this method does not exist, the code in the test says, "Ambiguous use of 'guous'
    /*
    public func guous() {
        print("guous")
    }
    */
    
    public func guous(x:Int=0) {
        print("guous x:")
    }
    
    public func guous(y:Int=1) {
        print("guous y:")
    }
    
}

class DefaultValuesTests: XCTestCase {
        
    func testDefaultValueAmbiguity() {
        let ambi = DefaultValues()
        ambi.guous()
    }
    
}

If the no-arg method doesn’t exist, the test does not compile. There is an error at the call site saying, “Ambiguous use of ‘guous’”. If the no-arg method does exist, it is preferred, and the code compiles and runs, even though the ambiguity still exists.

I think this should be classified as a compiler bug where it should be including no-arg methods and methods with default args to consider if they are ambiguous at the call site, not at the definition point.

-Kenny

···

On Jan 5, 2017, at 3:17 AM, Wagner Truppel via swift-users <swift-users@swift.org> wrote:

Exactly.

I would only add that renaming might not be the correct solution in some cases as, then, instances of the subclass would still be able to invoke the superclass method and that might not always be acceptable.

Wagner

On 5 Jan 2017, at 11:09, Frank O'Dwyer <frankodwyer@netscape.net> wrote:

I think there is a clear case for a warning here, in that the introduction of a default parameter value in such cases is in effect an empty promise and just introduces unreachable code. Indeed 'unreachable code' should perhaps be the warning.

This is because as soon as there is a foo() method, whether by inheritance or a definition in the same class, it renders it impossible to invoke foo(x:) with its default value in the normal way, or in any interesting way. In fact it makes no difference at all if the default is 0, 3, or 5, or not written in the first place. You now have to be explicit. Therefore, adding the default seems to provide no utility at all and can only lead to the confusion the OP is talking about, and bugs. Furthermore it seems like a very loud indication that the programmer expected something else to happen and it violates the 'principle of least surprise' for anyone else reading the code.

Given all that, it seems like the syntactic vinegar of having to write a pragma to get a clean compile would be appropriate. It alerts other readers of the code that 'here be dragons', and it tells the code author that they should probably achieve what they intend in another way. Most likely, by renaming foo(), or removing the useless default for foo(x:).

Cheers,
Frank

On 5 Jan 2017, at 07:58, Rien <Rien@Balancingrock.nl> wrote:

As you know. there is no ambiguity, no warnings needed.
(The parameter is part of the identifier of the function)

Imo, this request falls into the category “do as I think, not as I say”.

That is a discussion without end. Personally I am against ANY warnings of this kind. The reason is that I want my code to compile warnings free (default compiler behaviour) and I do not want an extra pragma in the code to instruct the compiler that when I am calling “foo()” I do indeed want to call “foo()”.

Regards,
Rien

Site: http://balancingrock.nl
Blog: http://swiftrien.blogspot.com
Github: http://github.com/Swiftrien
Project: http://swiftfire.nl

On 05 Jan 2017, at 03:29, Wagner Truppel via swift-users <swift-users@swift.org> wrote:

Hello,

I wasn’t sure whether to post this message here, at swift-dev, or at swift-evolution. so I’ll try here first. Hopefully it will get to the right group of people or, if not, someone will point me to the right mailing list.

I came across a situation that boils down to this example:

class Parent {
func foo() {
    print("Parent foo() called")
}
}

class Child: Parent {
func foo(x: Int = 0) {
    print("Child foo() called")
}
}

let c = Child()
c.foo() // prints "Parent foo() called"

I understand why this behaves like so, namely, the subclass has a method foo(x:) but no direct implementation of foo() so the parent’s implementation is invoked rather than the child's. That’s all fine except that it is not very intuitive.

I would argue that the expectation is that the search for an implementation should start with the subclass (which is does) but should look at all possible restrictions of parent implementations, including the restriction due to default values.

At the very least, I think the compiler should emit a warning or possibly even an error.

Thanks for reading.
Cheers,

Wagner
_______________________________________________
swift-users mailing list
swift-users@swift.org
https://lists.swift.org/mailman/listinfo/swift-users

_______________________________________________
swift-users mailing list
swift-users@swift.org
https://lists.swift.org/mailman/listinfo/swift-users