Skip to content

[AutoDiff] Enable @derivative attribute qualified declaration names. #28892

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

Enable qualified declaration names in @derivative attribute, just like
@transpose attribute.

DerivativeAttr now stores a base type TypeRepr *, which is non-null for
parsed attributes that reference a qualified original declaration.

Add TypeResolutionFlags::AllowModule flag to enable module lookup via
TypeChecker::lookupMember given a ModuleType.

Add tests for type-qualified and module-qualified declaration names.

Resolves TF-1058.


Examples:

func foo(_ x: Float) -> Float { x }

// 1. Module qualification.
// Immediately useful for derivative registration use cases: TF-1002.
// May be replaced by proper module qualification/`DeclNameRef` work:
// - https://forums.swift.org/t/pitch-fully-qualified-name-syntax/28482
// - https://forums.swift.org/t/what-the-eff-is-declnameref/31594
@derivative(of: MyModule.foo)
func vjpFoo(_ x: Float) -> (value: Float, pullback: (Float) -> Float) {
  (foo(x), { $0 })
}

struct Struct {
  func instanceMethod(_ x: Float) -> Float { x }
}
extension Struct {
  // 2. Type qualification.
  // Not very useful for `@derivative` attribute, because the original
  // declaration type context can always be inferred.
  // Currently useful for `@transpose` attribute until TF-1060.
  @derivative(of: Struct.instanceMethod)
  func vjpInstanceMethod(_ x: Float) -> (value: Float, pullback: (Float) -> Float) {
    (instanceMethod(x), { $0 })
  }
}

Enable qualified declaration names in `@derivative` attribute, just like
`@transpose` attribute.

`DerivativeAttr` now stores a base type `TypeRepr *`, which is non-null for
parsed attributes that reference a qualified original declaration.

Add `TypeResolutionFlags::AllowModule` flag to enable module lookup via
`TypeChecker::lookupMember` given a `ModuleType`.

Add tests for type-qualified and module-qualified declaration names.

Resolves TF-1058.
@dan-zheng dan-zheng added the tensorflow This is for "tensorflow" branch PRs. label Dec 20, 2019
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.

Thank you!

@@ -66,6 +66,9 @@ enum class TypeResolutionFlags : uint16_t {

/// Whether we should not produce diagnostics if the type is invalid.
SilenceErrors = 1 << 10,

/// Whether to allow module declaration types.
AllowModule = 1 << 11
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@brentdax: I added you as a reviewer because this ad-hoc support for module-qualified and type-qualified declaration names for @derivative and @transpose attributes overlaps with your work on module-qualified names.

Some context: the main "qualified name" use case for @derivative and @transpose is module-qualified names. Type-qualified names are no longer necessary after a @transpose type-checking rules revamp (TF-1060).

After syntax exists for your work on module-qualified names (e.g. Module::fooName), we can drop our ad-hoc support and switch to your work!

@dan-zheng
Copy link
Contributor Author

@swift-ci Please test

@dan-zheng
Copy link
Contributor Author

dan-zheng commented Dec 20, 2019

Merging to unblock progress. I'll cherry-pick this change to tensorflow branch.
Happy to address any review feedback later!

@dan-zheng dan-zheng merged commit 14ee6c5 into swiftlang:master Dec 20, 2019
@dan-zheng dan-zheng deleted the derivative-attr-qualified-decl branch December 20, 2019 05:06
dan-zheng added a commit to dan-zheng/swift that referenced this pull request Dec 20, 2019
Fix build error introduced in swiftlang#28892.

Caused by not re-running CI after swiftlang#28879
was merged.
@dan-zheng dan-zheng removed the tensorflow This is for "tensorflow" branch PRs. label Dec 20, 2019
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.

2 participants