Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[SR-8619] Reject bare references to cross-module property overloads in Swift 5 mode #51134

Closed
hamishknight opened this issue Aug 23, 2018 · 4 comments
Closed

[SR-8619] Reject bare references to cross-module property overloads in Swift 5 mode #51134

hamishknight opened this issue Aug 23, 2018 · 4 comments
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself regression swift 5.0 type checker Area → compiler: Semantic analysis

Comments

@hamishknight
Copy link
Collaborator

Previous ID SR-8619
Radar None
Original Reporter @hamishknight
Type Bug
Status Closed
Resolution Won't Do
Additional Detail from JIRA
Votes 0
Component/s Compiler
Labels Bug, 5.0Regression, TypeChecker
Assignee None
Priority Medium

md5: 04793bdded2833dcbbec9817d61fdd9c

Issue Description:

In Swift 4.1 we had the following overloading behaviour for a very specific case of cross-module overloading where one property was in an extension of a generic type, and the other property wasn't:

// Module A

public struct S<T> {
  public var foo = 0
}
// Module B

import A

extension S {
  var foo: String { return "" }
  func bar() {
    let x1: Int = foo // resolves to module A's `foo: Int`.
    let x2: String = foo // resolves to module B's `foo: String`.
    let x3 = foo // ambiguity error.
  }
}

In any other case (e.g if the type is non-generic, or if both properties are defined in extensions), we have this behaviour:

// Module A

public struct S {
  public var foo = 0
}
// Module B

import A

extension S {
  var foo: String { return "" }
  func bar() {
    let x1: Int = foo // error, cannot resolve to module A's `foo: Int`.
    let x2: String = foo // resolves to module B's `foo: String`.
    let x3 = foo // resolves to module B's `foo: String`.
  }
}

And this is also the behaviour that both examples will now exhibit in Swift 5 mode. The property from the current module always shadows the property from the other module.

IMO this behaviour is unintuitive – we should change the behaviour to act more like the behaviour of the first example in Swift 4.1. That is, we should reject bare reference to such property overloads as ambiguous and allow the user to disambiguate by type (and if we ever introduce syntax to disambiguate such cases by module, we could require the user to do that instead).

For more context, see https://forums.swift.org/t/should-we-allow-cross-module-overloading-of-variables-and-properties/15096.
@belkadan
Copy link
Contributor

Yeah, Swift 4.1 behavior definitely seems correct here! cc @slavapestov, @DougGregor

@belkadan
Copy link
Contributor

belkadan commented Feb 5, 2019

Where did we end up on this?

@hamishknight
Copy link
Collaborator Author

Apologies, I was planning on circling back to this after waiting to see if there was any more discussion on the forum post, but never got round to it. I am happy to work on an implementation, though I'm not sure whether more Swift Evolution input is required (bearing in mind it would be source breaking). Thoughts?

@hamishknight
Copy link
Collaborator Author

Okay, the more I think about this, the less convinced I am that this is the right solution. Assuming we eventually want some syntax for users to disambiguate cross-module property overloads by specifying the module they want (with the current module shadowing the other module by default like we do with global variables), then implementing this change would make it harder to transition to such a model due to the fact that the user would need to migrate their type inference based disambiguation to the new syntax. If we stick with the current implementation, there would be no such source breakage for introducing a syntax to disambiguate cross-module property overloads, as there's currently no other way to disambiguate.

Furthermore if we do go ahead with this change, then it would only really make sense to apply it to cross-module property overloads due to the fact that global variables can already be disambiguated using the 'module dot' syntax, therefore introducing an inconsistency. This change also isn't a general solution to the problem due to the fact that it relies on disambiguation by type; therefore it doesn't help cases where the properties defined by both modules have the same type.

For those reasons, I now believe we should stick to the current model, that is:

  • Global variables and properties defined in the current module shadow those declared in another module

  • For global variables, the other module's variable can be selected with the 'module dot' syntax

And hopefully we can look into introducing syntax that would allow the user to select another module's property overload, bringing things in line with global variables.

Does this sound reasonable? I have put together an implementation locally for cross-module property overloading based on type which I'm happy to submit if it's still felt that it's something we want.

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself regression swift 5.0 type checker Area → compiler: Semantic analysis
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@belkadan @hamishknight @AnthonyLatsis