Function overload resolution

I recently got bit by SR-2450 <https://bugs.swift.org/browse/SR-2450>, and
I’d like to try to fix it. However, I’ve never worked on the compiler
before and I have some questions:

1. Is this a reasonable first bug to tackle?
2. What resources are available for newcomers to the Swift project?
3. What will I need to learn about in order to address SR-2450?

Thanks,

Nevin

This appears to be resolved (in fact, I remember improving this some time ago). I get a much better diagnostic now

error: repl.swift:4:16: error: use of 'min' refers to instance method 'min()' rather than global function 'min' in module 'Swift'
        return min(1,2)
               ^

repl.swift:4:16: note: use 'Swift.' to reference the global function in module 'Swift'
        return min(1,2)
               ^
               Swift.

What version of Swift are you still seeing the bad diagnostic in?

~Robert Widmann

···

On Sep 24, 2017, at 12:03 PM, Nevin Brackett-Rozinsky via swift-dev <swift-dev@swift.org> wrote:

I recently got bit by SR-2450, and I’d like to try to fix it. However, I’ve never worked on the compiler before and I have some questions:

1. Is this a reasonable first bug to tackle?
2. What resources are available for newcomers to the Swift project?
3. What will I need to learn about in order to address SR-2450?

Thanks,

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

The new diagnostic is fine, the problem is that there should not be an
error at all. If there is only one function with the proper signature, the
compiler should not invent a non-existent ambiguity.

The situation I encountered involves functions of different arities, so it
should be straightforward to select the correct one, yet it still fails to
compile. I'd like to make it work.

Nevin

···

On Sunday, September 24, 2017, Robert Widmann <devteam.codafi@gmail.com> wrote:

This appears to be resolved (in fact, I remember improving this some time
ago). I get a much better diagnostic now

> error: repl.swift:4:16: error: use of 'min' refers to instance method
'min()' rather than global function 'min' in module 'Swift'
> return min(1,2)
> ^
>
> repl.swift:4:16: note: use 'Swift.' to reference the global function in
module 'Swift'
> return min(1,2)
> ^
> Swift.

What version of Swift are you still seeing the bad diagnostic in?

~Robert Widmann

> On Sep 24, 2017, at 12:03 PM, Nevin Brackett-Rozinsky via swift-dev < > swift-dev@swift.org <javascript:;>> wrote:
>
> I recently got bit by SR-2450, and I’d like to try to fix it. However,
I’ve never worked on the compiler before and I have some questions:
>
> 1. Is this a reasonable first bug to tackle?
> 2. What resources are available for newcomers to the Swift project?
> 3. What will I need to learn about in order to address SR-2450?
>
> Thanks,
>
> Nevin
> _______________________________________________
> swift-dev mailing list
> swift-dev@swift.org <javascript:;>
> https://lists.swift.org/mailman/listinfo/swift-dev

If either function had the correct signature and was being properly disambiguated there would not be a diagnostic popped. Can you provide an example of this?

~Robert Widmann

···

On Sep 24, 2017, at 8:58 PM, Nevin Brackett-Rozinsky <nevin.brackettrozinsky@gmail.com> wrote:

The new diagnostic is fine, the problem is that there should not be an error at all. If there is only one function with the proper signature, the compiler should not invent a non-existent ambiguity.

The situation I encountered involves functions of different arities, so it should be straightforward to select the correct one, yet it still fails to compile. I'd like to make it work.

Nevin

On Sunday, September 24, 2017, Robert Widmann <devteam.codafi@gmail.com <mailto:devteam.codafi@gmail.com>> wrote:
This appears to be resolved (in fact, I remember improving this some time ago). I get a much better diagnostic now

> error: repl.swift:4:16: error: use of 'min' refers to instance method 'min()' rather than global function 'min' in module 'Swift'
> return min(1,2)
> ^
>
> repl.swift:4:16: note: use 'Swift.' to reference the global function in module 'Swift'
> return min(1,2)
> ^
> Swift.

What version of Swift are you still seeing the bad diagnostic in?

~Robert Widmann

> On Sep 24, 2017, at 12:03 PM, Nevin Brackett-Rozinsky via swift-dev <swift-dev@swift.org <javascript:;>> wrote:
>
> I recently got bit by SR-2450, and I’d like to try to fix it. However, I’ve never worked on the compiler before and I have some questions:
>
> 1. Is this a reasonable first bug to tackle?
> 2. What resources are available for newcomers to the Swift project?
> 3. What will I need to learn about in order to address SR-2450?
>
> Thanks,
>
> Nevin
> _______________________________________________
> swift-dev mailing list
> swift-dev@swift.org <javascript:;>
> https://lists.swift.org/mailman/listinfo/swift-dev

func triple(_ x: Int) -> Int {
    return 3 * x
}

extension Int {
    func triple() -> Int {
        return triple(self) // Error here
    }
}

The error reads:

Playground execution failed:

error: Test.playground:5:16: error: use of 'triple' refers to instance
method 'triple()' rather than global function 'triple' in module
'__lldb_expr_52'
        return triple(self) // Error here
               ^
Test.playground:5:16: note: use '__lldb_expr_52.' to reference the global
function in module '__lldb_expr_52'
        return triple(self) // Error here
               ^
               __lldb_expr_52.

Notice where the error says, “use of 'triple' refers to instance method
'triple()' rather than global function 'triple'”.

Notice further that the instance method takes 0 arguments. In particular,
“self.triple()” is a valid way to call it, and “self.triple(self)” is not.

It is true that the instance method could be called as a type method with 1
argument, “Int.triple(self)”. However, “triple(self)” is not a valid way to
call the type method, which we can demonstrate by removing the global
function entirely:

extension Int {
    func triple() -> Int {
        return triple(self) // Error here
    }
}

This time the error reads:

Playground execution failed:

error: Test.playground:3:23: error: argument passed to call that takes no
arguments
        return triple(self)
                     ~^~~~~

So the compiler correctly recognizes that “triple(self)” is not a valid
call to the instance method. Indeed, it has the wrong arity. From there, it
seems to me a small step to reason that, in the case where a function with
the proper signature *does* exist, that it should be called.

• • •

Also, as a minor point, going back to the original code, notice there are
two diagnostic messages. The second one says, “use '__lldb_expr_52.' to
reference the global function”. However, that does not work, and indeed
every time I run the playground the number shown changes.

So it seems that in a playground, the diagnostic is incorrect, as the
proposed solution does not work. Is there in fact a way to specify the
module for a playground, so as to unambiguously call a global function?

• • •

In any case, the proper behavior seems clear-cut to my mind. The shorthand
for calling an instance method without “self.” should apply only if there
is a matching instance method to call. When there is no instance method
which could possibly match, but there is a global function that does, then
the unqualified call should resolve to the global function.

Nevin

···

On Sun, Sep 24, 2017 at 10:16 PM, Robert Widmann <devteam.codafi@gmail.com> wrote:

If either function had the correct signature and was being properly
disambiguated there would not be a diagnostic popped. Can you provide an
example of this?

~Robert Widmann

On Sep 24, 2017, at 8:58 PM, Nevin Brackett-Rozinsky < > nevin.brackettrozinsky@gmail.com> wrote:

The new diagnostic is fine, the problem is that there should not be an
error at all. If there is only one function with the proper signature, the
compiler should not invent a non-existent ambiguity.

The situation I encountered involves functions of different arities, so it
should be straightforward to select the correct one, yet it still fails to
compile. I'd like to make it work.

Nevin

On Sunday, September 24, 2017, Robert Widmann <devteam.codafi@gmail.com> > wrote:

This appears to be resolved (in fact, I remember improving this some time
ago). I get a much better diagnostic now

> error: repl.swift:4:16: error: use of 'min' refers to instance method
'min()' rather than global function 'min' in module 'Swift'
> return min(1,2)
> ^
>
> repl.swift:4:16: note: use 'Swift.' to reference the global function in
module 'Swift'
> return min(1,2)
> ^
> Swift.

What version of Swift are you still seeing the bad diagnostic in?

~Robert Widmann

> On Sep 24, 2017, at 12:03 PM, Nevin Brackett-Rozinsky via swift-dev < >> swift-dev@swift.org> wrote:
>
> I recently got bit by SR-2450, and I’d like to try to fix it. However,
I’ve never worked on the compiler before and I have some questions:
>
> 1. Is this a reasonable first bug to tackle?
> 2. What resources are available for newcomers to the Swift project?
> 3. What will I need to learn about in order to address SR-2450?
>
> Thanks,
>
> Nevin
> _______________________________________________
> swift-dev mailing list
> swift-dev@swift.org
> https://lists.swift.org/mailman/listinfo/swift-dev

func triple(_ x: Int) -> Int {
    return 3 * x
}

extension Int {
    func triple() -> Int {
        return triple(self) // Error here
    }
}

The error reads:

Playground execution failed:
error: Test.playground:5:16: error: use of 'triple' refers to instance method 'triple()' rather than global function 'triple' in module '__lldb_expr_52'
        return triple(self) // Error here
               ^
Test.playground:5:16: note: use '__lldb_expr_52.' to reference the global function in module '__lldb_expr_52'
        return triple(self) // Error here
               ^
               __lldb_expr_52.

Notice where the error says, “use of 'triple' refers to instance method 'triple()' rather than global function 'triple'”.

Notice further that the instance method takes 0 arguments. In particular, “self.triple()” is a valid way to call it, and “self.triple(self)” is not.

It is true that the instance method could be called as a type method with 1 argument, “Int.triple(self)”. However, “triple(self)” is not a valid way to call the type method, which we can demonstrate by removing the global function entirely:

extension Int {
    func triple() -> Int {
        return triple(self) // Error here
    }
}

This time the error reads:

Playground execution failed:
error: Test.playground:3:23: error: argument passed to call that takes no arguments
        return triple(self)
                     ~^~~~~

So the compiler correctly recognizes that “triple(self)” is not a valid call to the instance method. Indeed, it has the wrong arity. From there, it seems to me a small step to reason that, in the case where a function with the proper signature *does* exist, that it should be called.

• • •

Also, as a minor point, going back to the original code, notice there are two diagnostic messages. The second one says, “use '__lldb_expr_52.' to reference the global function”. However, that does not work, and indeed every time I run the playground the number shown changes.

So it seems that in a playground, the diagnostic is incorrect, as the proposed solution does not work. Is there in fact a way to specify the module for a playground, so as to unambiguously call a global function?

That doesn’t work at the REPL because LLDB is handing you a new module context every time you execute an expression. When you factor this into a playground/framework/executable you can disambiguate with the name of the module.

• • •

In any case, the proper behavior seems clear-cut to my mind. The shorthand for calling an instance method without “self.” should apply only if there is a matching instance method to call. When there is no instance method which could possibly match, but there is a global function that does, then the unqualified call should resolve to the global function.

You may be able to get away with this by teaching lookup for overloads to pull in global declarations, but you’ll need to adjust the ranking to penalize this as well. This is assuming there aren’t scoping issues we’re ignoring with a rule that, at first blush, seems like common sense. I’m not sure this is StarterBug material - but updating the diagnostic to be more clear would definitely be.

~Robert Widmann

···

On Sep 25, 2017, at 10:01 AM, Nevin Brackett-Rozinsky <nevin.brackettrozinsky@gmail.com> wrote:

Nevin

On Sun, Sep 24, 2017 at 10:16 PM, Robert Widmann <devteam.codafi@gmail.com <mailto:devteam.codafi@gmail.com>> wrote:
If either function had the correct signature and was being properly disambiguated there would not be a diagnostic popped. Can you provide an example of this?

~Robert Widmann

On Sep 24, 2017, at 8:58 PM, Nevin Brackett-Rozinsky <nevin.brackettrozinsky@gmail.com <mailto:nevin.brackettrozinsky@gmail.com>> wrote:

The new diagnostic is fine, the problem is that there should not be an error at all. If there is only one function with the proper signature, the compiler should not invent a non-existent ambiguity.

The situation I encountered involves functions of different arities, so it should be straightforward to select the correct one, yet it still fails to compile. I'd like to make it work.

Nevin

On Sunday, September 24, 2017, Robert Widmann <devteam.codafi@gmail.com <mailto:devteam.codafi@gmail.com>> wrote:
This appears to be resolved (in fact, I remember improving this some time ago). I get a much better diagnostic now

> error: repl.swift:4:16: error: use of 'min' refers to instance method 'min()' rather than global function 'min' in module 'Swift'
> return min(1,2)
> ^
>
> repl.swift:4:16: note: use 'Swift.' to reference the global function in module 'Swift'
> return min(1,2)
> ^
> Swift.

What version of Swift are you still seeing the bad diagnostic in?

~Robert Widmann

> On Sep 24, 2017, at 12:03 PM, Nevin Brackett-Rozinsky via swift-dev <swift-dev@swift.org <>> wrote:
>
> I recently got bit by SR-2450, and I’d like to try to fix it. However, I’ve never worked on the compiler before and I have some questions:
>
> 1. Is this a reasonable first bug to tackle?
> 2. What resources are available for newcomers to the Swift project?
> 3. What will I need to learn about in order to address SR-2450?
>
> Thanks,
>
> Nevin
> _______________________________________________
> swift-dev mailing list
> swift-dev@swift.org <>
> https://lists.swift.org/mailman/listinfo/swift-dev

This is definitely a change that would need to go through swift-evolution. There are a number of potential issues and possibilities for breaking existing code that’s relying on this shadowing behavior, intentionally or unintentionally.

Jordan

···

On Sep 26, 2017, at 18:02, Robert Widmann via swift-dev <swift-dev@swift.org> wrote:

On Sep 25, 2017, at 10:01 AM, Nevin Brackett-Rozinsky <nevin.brackettrozinsky@gmail.com <mailto:nevin.brackettrozinsky@gmail.com>> wrote:

func triple(_ x: Int) -> Int {
    return 3 * x
}

extension Int {
    func triple() -> Int {
        return triple(self) // Error here
    }
}

The error reads:

Playground execution failed:
error: Test.playground:5:16: error: use of 'triple' refers to instance method 'triple()' rather than global function 'triple' in module '__lldb_expr_52'
        return triple(self) // Error here
               ^
Test.playground:5:16: note: use '__lldb_expr_52.' to reference the global function in module '__lldb_expr_52'
        return triple(self) // Error here
               ^
               __lldb_expr_52.

Notice where the error says, “use of 'triple' refers to instance method 'triple()' rather than global function 'triple'”.

Notice further that the instance method takes 0 arguments. In particular, “self.triple()” is a valid way to call it, and “self.triple(self)” is not.

It is true that the instance method could be called as a type method with 1 argument, “Int.triple(self)”. However, “triple(self)” is not a valid way to call the type method, which we can demonstrate by removing the global function entirely:

extension Int {
    func triple() -> Int {
        return triple(self) // Error here
    }
}

This time the error reads:

Playground execution failed:
error: Test.playground:3:23: error: argument passed to call that takes no arguments
        return triple(self)
                     ~^~~~~

So the compiler correctly recognizes that “triple(self)” is not a valid call to the instance method. Indeed, it has the wrong arity. From there, it seems to me a small step to reason that, in the case where a function with the proper signature *does* exist, that it should be called.

• • •

Also, as a minor point, going back to the original code, notice there are two diagnostic messages. The second one says, “use '__lldb_expr_52.' to reference the global function”. However, that does not work, and indeed every time I run the playground the number shown changes.

So it seems that in a playground, the diagnostic is incorrect, as the proposed solution does not work. Is there in fact a way to specify the module for a playground, so as to unambiguously call a global function?

That doesn’t work at the REPL because LLDB is handing you a new module context every time you execute an expression. When you factor this into a playground/framework/executable you can disambiguate with the name of the module.

• • •

In any case, the proper behavior seems clear-cut to my mind. The shorthand for calling an instance method without “self.” should apply only if there is a matching instance method to call. When there is no instance method which could possibly match, but there is a global function that does, then the unqualified call should resolve to the global function.

You may be able to get away with this by teaching lookup for overloads to pull in global declarations, but you’ll need to adjust the ranking to penalize this as well. This is assuming there aren’t scoping issues we’re ignoring with a rule that, at first blush, seems like common sense. I’m not sure this is StarterBug material - but updating the diagnostic to be more clear would definitely be.

Perhaps I am misunderstanding, but I don’t see how it is possible for code
to be relying on this in a manner that would be affected by what’s being
discussed.

*Things that will change with this bug fix:*

If there are both a local function and a global function with the base name
being called, and the local function’s signature does not match the
call-site but the global function’s does, that is currently a compiler
error, but after the bug is fixed the global function will be called.

*Things that will stay the same:*

1) If there is only a global function and no local function with the base
name being called, and the global function’s signature matches the
call-site, currently the global function is called, and it will still be
called after the bug is fixed.

2) If there is only a local function and no global function with the base
name being called, and the local function’s signature matches the
call-site, currently the local function is called, and it will still be
called after the bug is fixed.

3) If there is neither a local function nor a global function with the base
name being called, currently that is a compiler error, and it will still be
one after the bug is fixed.

4) If there are both a local function and a global function with the base
name being called, but neither of their signatures match the call-site,
currently that is a compiler error, and it will still be one after the bug
is fixed.

5) If there are both a local function and a global function with the base
name being called, and the local function’s signature matches the
call-site, the local function is called currently, and it will still be
called after the bug is fixed.

• • •

The only change here is that something which is a compiler error today,
will not be a compiler error once the bug is fixed. There cannot possibly
be any valid code which relies on the current behavior, because the current
behavior is a compiler error.

Am I missing something?

Nevin

···

On Wed, Sep 27, 2017 at 12:23 PM, Jordan Rose <jordan_rose@apple.com> wrote:

This is *definitely* a change that would need to go through
swift-evolution. There are a number of potential issues and possibilities
for breaking existing code that’s relying on this shadowing behavior,
intentionally or unintentionally.

Jordan