Skip to content

[AutoDiff] Change TangentVector memberwise initializer access level. #28908

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

Merged
merged 2 commits into from
Dec 21, 2019

Conversation

dan-zheng
Copy link
Contributor

@dan-zheng dan-zheng commented Dec 20, 2019

Previously, memberwise initializers of synthesized TangentVector structs were
internal by default. Now, they are public by default, improving usability.

Note that public synthesized memberwise initializers have no precedent in
Swift. For normal public structs, it's possible to manually define a public
memberwise initializer, skipping synthesis. However, since TangentVector
structs are always synthesized with a memberwise initializer, manually defining
a public memberwise initializer in an extension leads to a redeclaration error.

A broader discussion on memberwise initializer access levels may be
worthwhile.

Resolves TF-1077.
Note that public synthesized memberwise initializers have no precedent in
Swift. For normal public structs, it's possible to manually define a public
memberwise initializer, skipping synthesis. However, since TangentVector
structs are always synthesized with a memberwise initializer, manually defining
a public memberwise initializer in an extension leads to a redeclaration error:

$ cat foo.swift
public struct Foo: Differentiable {}
extension Foo.TangentVector {
  public init() {}
}

$ swift foo.swift
foo.swift:3:10: error: invalid redeclaration of synthesized memberwise 'init()'
  public init() {}
         ^

A broader discussion on memberwise initializer access levels may be
worthwhile.

Resolves TF-1077. Unblocks a concrete use case: cc @eaplatanios.

Change the access level of synthesized `TangentVector` struct memberwise
initializers to match the access level of the `TangentVector` struct itself.

Previously, memberwise initializers of public synthesized `TangentVector`
structs were internal. Now, they are public, improving usability.

Note that public synthesized memberwise initializers have no precedent in
Swift. For normal public structs, it's possible to manually define a public
memberwise initializer, skipping synthesis. However, since `TangentVector`
structs are always synthesized with a memberwise initializer, manually defining
a public memberwise initializer in an extension leads to a redeclaration error.

A broader discussion on memberwise initializer access levels may be
worthwhile.

Resolves TF-1077.
@dan-zheng dan-zheng added the tensorflow This is for "tensorflow" branch PRs. label Dec 20, 2019
@dan-zheng dan-zheng requested review from rxwei and marcrasi December 20, 2019 20:30
@dan-zheng
Copy link
Contributor Author

@swift-ci Please clean test tensorflow

@dan-zheng
Copy link
Contributor Author

@swift-ci Please test tensorflow

@dan-zheng
Copy link
Contributor Author

@swift-ci Please test tensorflow linux

@dan-zheng
Copy link
Contributor Author

@swift-ci Please test tensorflow

@dan-zheng dan-zheng merged commit df6be21 into swiftlang:tensorflow Dec 21, 2019
@dan-zheng dan-zheng deleted the TF-1077 branch December 21, 2019 00:30
@porterchild
Copy link

Is this still in effect? I'm currently getting a

'MyType.TangentVector' initializer is inaccessible due to 'internal' protection level

compiler error, and I get the

invalid redeclaration of synthesized memberwise 'init()'

error if I try to make a public init in an extension. (using 0.11 Release)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tensorflow This is for "tensorflow" branch PRs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants