Skip to content

[AutoDiff] Revamp derivative type calculation using llvm::Expected. #31727

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

Conversation

dan-zheng
Copy link
Contributor

@dan-zheng dan-zheng commented May 12, 2020

Create DerivativeFunctionTypeError representing all potential derivative
function type calculation errors.

Make AnyFunctionType::getAutoDiffDerivativeFunctionLinearMapType return
llvm::Expected<AnyFunctionType *>. This is much safer and caller-friendly
than performing assertions.

Delete hacks in @differentiable and @derivative attribute type-checking
for verifying that Differentiable.TangentVector type witnesses are valid:
this is no longer necessary.

Robust fix for TF-521: invalid Differentiable conformances during
@derivative attribute type-checking.

Resolves SR-12793: bad interaction between @differentiable and @derivative
attribute type-checking and Differentiable derived conformances.

Todos

  • Requestify AnyFunctionType::getAutoDiffDerivativeFunctionType and AnyFunctionType::getAutoDiffDerivativeFunctionLinearMapType to cache results.

Example

import _Differentiation

struct Struct: Differentiable {
  var x: Float

  @differentiable
  func method() -> Float { x }

  mutating func move(along direction: TangentVector) {}
}

Before (the bad interaction):

$ swiftc test.swift
test.swift:9:39: error: reference to invalid associated type 'TangentVector' of type 'Struct'
  mutating func move(along direction: TangentVector) {}
                                      ^

After: no error.

@dan-zheng dan-zheng requested review from rxwei, marcrasi and CodaFi May 12, 2020 08:57
Comment on lines -3394 to -3398
// Try to get the `TangentVector` type witness, in case the conformance has
// not been fully checked.
Type tanType = conf.getTypeWitnessByName(type, ctx.Id_TangentVector);
if (tanType.isNull() || tanType->hasError())
return ProtocolConformanceRef();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@CodaFi: the removal of this code should address your concerns from https://github.com/apple/swift/pull/30936/files#r406541238:

This mix of semantic and syntactico-semantic checks doesn't feel right. Is the idea that you're trying to check for a conformance to this protocol, or to a well-formed conformance of this protocol?

We directly use TypeChecker::conformsToProtocol now, which is syntactic. Semantic checks (requesting TangentVector type witnesses) are performed and diagnosed on-demand.

@dan-zheng
Copy link
Contributor Author

@swift-ci Please test

Create `DerivativeFunctionTypeError` representing all potential derivative
function type calculation errors.

Make `AnyFunctionType::getAutoDiffDerivativeFunctionLinearMapType` return
`llvm::Expected<AnyFunctionType *>`. This is much safer and caller-friendly
than performing assertions.

Delete hacks in `@differentiable` and `@derivative` attribute type-checking
for verifying that `Differentiable.TangentVector` type witnesses are valid:
this is no longer necessary.

Robust fix for TF-521: invalid `Differentiable` conformances during
`@derivative` attribute type-checking.

Resolves SR-12793: bad interaction between `@differentiable` and `@derivative`
attribute type-checking and `Differentiable` derived conformances.
@dan-zheng dan-zheng force-pushed the revamp-derivative-type-calculation branch from cb2a709 to 3bdfd50 Compare May 12, 2020 09:04
@dan-zheng
Copy link
Contributor Author

@swift-ci Please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - cb2a709f55de1cc984bcf648cb61d6aab9ca5ada

@dan-zheng
Copy link
Contributor Author

Merging now to unblock progress, I'm happy to address any feedback later!
I plan to address #31727 (comment) in a follow-up.

@dan-zheng dan-zheng merged commit 8500cf8 into swiftlang:master May 12, 2020
@dan-zheng dan-zheng deleted the revamp-derivative-type-calculation branch May 12, 2020 13:09
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