Allow referencing outer functions when inner functions share base name

Consider the following snippet:

func foo(x1: Int) {}

struct S {
    func foo(x2: Int) {}
    func method() {
        func foo(x3: Int) {}
        let closure = {
            func foo(x4: Int) {}
            foo(x1: 0)
            foo(x2: 0)
            foo(x3: 0)
            foo(x4: 0)
        }
    }
}

In Swift today, the first three calls to foo fail to compile with a complaint about the wrong argument label ("expected 'x4:'"). I found SR-7018 wherein @jrose confirms that this is the intended behavior.

As I've worked on an implementation for compound variable names, I came across this behavior. Given the relatively sparse usage of nested functions, it's not surprising that this behavior has not caused much trouble.

However, I do worry about it's consequence in the midst of compound variable names, since it would potentially make compound names behave subtly differently from simple names. E.g.,

func fetchImage(from url: URL, then completion(image:): (UIImage) -> Void) -> {
  let completion(data:): (Data) -> Void = {
    completion(image: makeImage(from: $0)) // Error: incorrect argument label
  }
  fetchData(from: url, completion: completion(data:))
}

I'm interested in the community's feedback on whether this behavior is more confusing in the context of compound names, or if it seems unlikely to be an issue in practice.

1 Like

Does using ModuleName.foo instead get you to foo(x1:)?

1 Like

Yeah, you can reference Module.foo(x1:), and self.foo(x2:) works as well. AFAIK there's no way to get at foo(x3:) within closure, but the double-nested functions are unusual enough that I'm not particularly troubled by being unable to access shadowed base names. With compound variable names, though, the points at which compound names can be introduced increases drastically (such as function parameters in the example above), so this sort of shadowing may be more problematic.

Typo? The first three calls give an error, the fourth call to foo(x4:) is fine.


  let completion(data:): (Data) -> Void = {
    completion(image: makeImage(from: $0)) // Error: incorrect argument label
  }

I don't get why should this be an error...? Swift's let is not recursive from what I can tell:

let f : () -> () = { f() } // error: variable used within its own initial value
1 Like

Yes, thank you, updated appropriately! :man_facepalming:

Yeah, we could probably massage the parser and lookup so that this example makes sense to the compiler, with the caveat that we'd have to give up on this (7 year old!) FIXME :sweat_smile:

// FIXME: We want this to work: "var x = { x() }", but for now it's better 
// to disallow it than to crash.

But the issue still arises even if we allow recursive closures, in ways that seem much more natural than the double-nested function:

func fetchImage(
  from url: URL, 
  then completion(image:): (UIImage) -> Void, 
  onError completion(error:): (Error) -> Void
) -> {
  let completion(data:): (Data) -> Void = {
    completion(image: makeImage(from: $0))
  }
  fetchData(from: url) { error, data in
    if let error = error {
      completion(error: error) // Incorrect argument label!
    } else if let data = data {
      completion(data: data)
    } else {
      assertionFailure("No error and no data!")
    }
  }
}

I suspect that changing that behavior will break some code relying on the interaction of shadowing and closures. I don't think fixing the FIXME is worth that breakage. Since local functions are easily written, you can tie the knot yourself. :man_shrugging:

Is this meaningfully different from SR-7018? IMO it is the same problem, but in a slightly different setting. I think completion(image:) should not behave as if it is in the same scope as completion(data:) because there is an unbalanced opening brace between them, and braces indicate scopes. "Braces indicate scopes" is a simpler mental rule than "braces indicate scopes except when they are the main braces for a function declaration". And different scopes implies shadowing, not addition to overload set (as per SR-7018).

That said here's the behavior for slightly similar code that you can write today:

func g(_ f: () -> ()) {
    func f(x: Int) {}
    f() // okay
    f(x: 10) // error: argument passed to call that takes no arguments
}

which has me somewhat shook. :open_mouth: (EDIT: Confirmed that this is a bug with Doug. Filed https://bugs.swift.org/browse/SR-13306)

2 Likes

Agreed.

Depends on your definition of "meaningful." My concern arises from two (perhaps unfounded) concerns:

  1. Local functions/closures with the same base name and different argument names will become more common if compound variable names become part of the language.
  2. The 'inline' spelling of compound-named variables implies that
var base(arg1:arg2:): (Int, String) -> Void
var base(arg3:arg4:): (Int, String) -> Void

should behave more like

var baseWithArg1Arg2: (Int, String) -> Void
var baseWithArg3Arg4: (Int, String) -> Void

than

func base(arg1: Int, arg2: String) {}
func base(arg3: Int, arg4: String) {}

I'm hoping this thread will shed some light on whether others think (1) and (2) are true in a world with compound-named variables.

Phew—I didn't know how to respond to that other than "oh, dear"!

This is essentially the same as:

SR–0456 Confusing build error when calling 'max' function within 'extension Int'
SR–1772 File-level function with the same name as instance function not picked up by compiler
SR–2450 Wrong min() function is selected for call
SR–4466 Extra arguments error for valid call to max
SR–4660 Methods on self should not block unambiguous calls to free functions
SR–7018 Overload resolution fails for nested function
SR–7143 Compiler chooses custom function over foundation function

…and probably more that I didn’t find at a quick search.

I’ve been meaning to start a thread about fixing this. As I see it, the correct solution is to change how the compiler matches candidates for a call-site.

Specifically, when expanding its search scope, the compiler should look at the full compound name including all argument labels, not just the base name, and it should not stop until it finds a candidate which could plausibly match the full name.

Declarations with the same base name but non-matching argument labels can still be identified for the purpose of generating diagnostics, but they should not be considered potential matches, and thus the search scope should continue past them.

Only a declaration which, using Swift’s rules for matching call-site arguments to function parameters by argument label, could potentially be a match based on its full compound name, should be considered a candidate.

When such a candidate matching the full compound name is found, then the search scope can stop expanding and any such candidates found there can be processed for type resolution as usual.

4 Likes

Thank you for pulling up all those examples!

Right, this would bring us in line with the "argument labels are part of the name" model that SE-0111 laid the groundwork for. I'm hoping this change can be made in a source-compatible way—I haven't thought of any situation in which this extension of the lookup rules would break existing code.

FWIW, I was wrong about this. Writing f(x3:)(0) compiles just fine. If f(x3:) were instead defined as func foo() {}, though, I can't think of a way around the current shadowing rules.

1 Like

I concur. Any existing code which compiles today, must necessarily already have a valid candidate that matches the call-site successfully, and no smaller scopes do.

Tightening the rules to account for argument labels cannot introduce any new candidates at or below the same scope, and it can only rule out incompatible candidates. So the existing valid candidate will not be eliminated, and no new candidates will be added.

Code that works today will continue to work the same way with this improvement.

6 Likes

This seems also related to

let max = max(1, 2) // Variable used within its own initial value
let max = 1
let max1 = max(1, 2) // Cannot call value of non-function type 'Int'

https://bugs.swift.org/browse/SR-1846

Terms of Service

Privacy Policy

Cookie Policy