Skip to content

inherit required protocols during TangentVector synthesis #34893

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 3 commits into from
Dec 1, 2020

Conversation

marcrasi
Copy link

We have been running into a limitation with TangentVector synthesis. Here's a proposed solution and implementation.

The Problem

If you have a protocol P that refines Differentiable and that constrains the TangentVector to conform to something that can be derived, the compiler can't derive a conformance to P.

e.g.

protocol P: Differentiable where TangentVector: Encodable {}

struct S: P {
  var x: Float
}

/// error: <Cell 1>:3:8: error: type 'S.TangentVector' does not conform to protocol 'Encodable'
/// struct S: P {
///       ^

Proposed Solution

When deriving the TangentVector, look at the conformance the triggered the derivation, find conformance constraints on TangentVector implied by that conformance, and make TangentVector inherit the required protocols.

One nice property of this solution is that it automatically figures out that TangentVector must conform to AdditiveArithmetic and Differentiable, so these conformances no longer need to be hardcoded in the derivation.

Copy link
Contributor

@rxwei rxwei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Suggest documenting this behavior in the manifesto and the proposal.

Copy link
Contributor

@dan-zheng dan-zheng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice generalization!

To confirm, I wonder if this is a workaround for associated type inference changes in #32578?
Some more context: tensorflow/swift-models#620.

@marcrasi
Copy link
Author

To confirm, I wonder if this is a workaround for associated type inference changes in #32578?
Some more context: tensorflow/swift-models#620.

No I think it is unrelated. Those changes look like they affect cases where a protocol constrains the TangentVector to be equal to a specified type. This PR is for when a protocol constrains the TangentVector to conform to something.

@marcrasi
Copy link
Author

Suggest documenting this behavior in the manifesto and the proposal.

I added a line to the manifesto in this PR, thanks. I'll add a corresponding line to the proposal in a separate PR because that's in a separate repo.

@marcrasi
Copy link
Author

marcrasi commented Dec 1, 2020

@swift-ci please test

@marcrasi marcrasi merged commit 4e048bf into swiftlang:main Dec 1, 2020
@marcrasi marcrasi deleted the make-it-automatically-inherit branch December 1, 2020 07:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants