[AutoDiff] [Sema] Limit implicit `@differentiable` attribute creation. #33776

dan-zheng · 2020-09-03T00:05:16Z

During protocol witness matching for a protocol requirement with
@differentiable attributes, implicit @differentiable attributes may be
created for the witness under specific conditions (when the witness has a
@differentiable attribute with superset differentiability parameters, or
when the witness has less-than-public visibility). This avoids requiring users
to explicitly write unnecessary @differentiable attributes.

Do not generate implicit @differentiable attributes for protocol witnesses
when the protocol conformance is declared from a separate file from the witness.

Otherwise, compilation of the file containing the conformance creates references
to external symbols for the implicit @differentiable attributes, even though
no such symbols exist.

Resolves SR-13455.

Example:

// file1.swift
import _Differentiation

protocol P1: Differentiable {
  @differentiable(wrt: self)
  func callAsFunction(_ input: Float) -> Float
}

protocol P2: P1 {}

extension P2 {
  // Previously, protocol witness matching created this implicit attribute:
  // @differentiable(wrt: self)
  @differentiable(wrt: (self, input))
  public func callAsFunction(_ input: Float) -> Float {
    return input
  }
}

// main.swift
import _Differentiation

struct S: P2 {}

Before:

$ swiftc file1.swift main.swift
Undefined symbols for architecture x86_64:
  "_AD__$s4main2P2PAAE14callAsFunctionyS2fF_PUSRS4main2P2Rzl", referenced from:
      _AD__$s4main1SVAA2P1A2aDP14callAsFunctionyS2fFTW_jvp_US in main-bddbaa.o
      _AD__$s4main1SVAA2P1A2aDP14callAsFunctionyS2fFTW_vjp_US in main-bddbaa.o
ld: symbol(s) not found for architecture x86_64
<unknown>:0: error: link command failed with exit code 1 (use -v to see invocation)

After:

$ swiftc file1.swift main.swift
main.swift:4:8: error: type 'S' does not conform to protocol 'P1'
struct S: P2 {}
       ^
file1.swift:14:15: note: instance method 'callAsFunction' must have explicit '@differentiable(wrt: self)' attribute to satisfy requirement instance method 'callAsFunction' (in protocol 'P1') because it is declared in a different file than the conformance of 'S' to 'P1'
  public func callAsFunction(_ input: Float) -> Float {
              ^
         @differentiable(wrt: self)
file1.swift:5:8: note: protocol requires function 'callAsFunction' with type '(Float) -> Float'
  func callAsFunction(_ input: Float) -> Float
       ^

dan-zheng · 2020-09-03T00:05:27Z

@swift-ci Please smoke test

texasmichelle

🎉 Thank you for this fix!

rxwei

+1 on fixing the crasher, but IMO the semantics implied here could be improved. Why should a refining protocol declare an explicit @differentiable attribute when it can be inherited?

In the example you gave:

protocol P1: Differentiable {
  @differentiable(wrt: self)
  func callAsFunction(_ input: Float) -> Float
}
protocol P2: P1 {}
extension P2 {
  @differentiable(wrt: (self, input))
  public func callAsFunction(_ input: Float) -> Float {
    return input
  }
}

I don't see any issues with generating the derivative for P2. callAsFunction(_:) with respect to self, because any call to the derivative would need to call through protocol dispatch.

rxwei · 2020-09-03T00:55:41Z

The error message also seems non-standard:

file1.swift:14:15: note: instance method 'callAsFunction' must have explicit '@differentiable(wrt: self)' attribute to satisfy requirement instance method 'callAsFunction' (in protocol 'P1') because it is declared in a different file than the conformance of 'S' to 'P1'
  public func callAsFunction(_ input: Float) -> Float {
              ^
         @differentiable(wrt: self)

This seems to be suggesting that it's the other file's responsibility to fix, while providing no ways to actually fix it within the current file. What if this is imported from a different module?

dan-zheng · 2020-09-03T01:37:30Z

+1 on fixing the crasher, but IMO the semantics implied here could be improved. Why should a refining protocol declare an explicit @differentiable attribute when it can be inherited?

Ah, good question. Here's the original example again:

// file1.swift
import _Differentiation

protocol P1: Differentiable {
  @differentiable(wrt: self)
  func callAsFunction(_ input: Float) -> Float
}

protocol P2: P1 {}

extension P2 {
  @differentiable(wrt: (self, input))
  public func callAsFunction(_ input: Float) -> Float {
    return input
  }
}

// main.swift
import _Differentiation

struct S: P2 {}

The issue is the trigger of implicit @differentiable attribute creation.

Currently, implicit @differentiable attribute creation happens during protocol witness matching, e.g. when type-checking the S: P2 conformance in main.swift. No implicit @differentiable attribute creation happens when type-checking file1.swift, since there's no conformance in the file.

It would be nice in theory to make P2.callAsFunction inherit the @differentiable(wrt: self) attribute from P1.callAsFunction, but I'm not sure how to do this easily in practice. It seems to require moving implicit @differentiable attribute creation from protocol witness matching to @differentiable attribute type-checking: when checking @differentiable(wrt: (self, input)) on P2.callAsFunction, see that P2.callAsFunction "matches" P1.callAsFunction and thus should inherit @differentiable(wrt: self) from P1.callAsFunction. This would involve some reimplementation of protocol witness matching, which isn't trivial.

Does this make sense? Do you have other ideas?

dan-zheng · 2020-09-03T01:49:08Z

The error message also seems non-standard:
file1.swift:14:15: note: instance method 'callAsFunction' must have explicit '@differentiable(wrt: self)' attribute to satisfy requirement instance method 'callAsFunction' (in protocol 'P1') because it is declared in a different file than the conformance of 'S' to 'P1'
  public func callAsFunction(_ input: Float) -> Float {
              ^
         @differentiable(wrt: self)
This seems to be suggesting that it's the other file's responsibility to fix, while providing no ways to actually fix it within the current file. What if this is imported from a different module?

That's a great point. How about this message instead:

file1.swift:14:15: note: candidate is missing explicit '@differentiable(wrt: self)' attribute to satisfy requirement 'callAsFunction' (in protocol 'P1') because it is declared in a different file than the conformance of 'S' to 'P1'
  public func callAsFunction(_ input: Float) -> Float {
              ^
         @differentiable(wrt: self)

The phrasing candidate is missing explicit '@differentiable(wrt: self)' attribute is more like an observation and places less responsibility on the other file.

Edit: we discussed offline and agreed this message is better.

Context: error messages like these are precedented for other protocol witness mismatch kinds. Here's an mutating-ness mismatch example:

// file1.swift
protocol P1 {
  func method()
}

protocol P2: P1 {}
extension P2 {
  mutating func method() {}
}

// main.swift
struct S: P2 {}

$ swiftc file1.swift main.swift
main.swift:4:8: error: type 'S' does not conform to protocol 'P1'
struct S: P2 {}
       ^
file1.swift:8:17: note: candidate is marked 'mutating' but protocol does not allow it
  mutating func method() {}
                ^
file1.swift:3:8: note: protocol requires function 'method()' with type '() -> ()'
  func method()
       ^

dan-zheng · 2020-09-03T02:27:16Z

@swift-ci Please smoke test

rxwei · 2020-09-03T04:12:56Z

We discussed a bit offline, but I feel we've missed an important issue in today's semantics as well as the semantics proposed in this PR. I think there are multiple aspects to the issue that this PR is trying to mitigate, but I think the root problem is the fact that conformances are implicitly changing declarations in protocol extensions. Let me dive into this with examples.

Case 1: Candidates in protocol extensions that do not match

First, should declarations in a protocol extension that almost matches a protocol requirement be modified so that it can be treat as a requirement? Similar to what you posted above, such an example would be:

protocol P1 {
  func method()
}

protocol P2: P1 {}
extension P2 {
  mutating func method() {}
}

struct S: P2 {}

In this example, the protocol extension method does not (but almost!) matches the protocol requirement. Note that it has nothing to do with whether the conformance is declared in the same file or not. The example above results in compilation errors similar to the ones you've shown. Therefore, a similar AD example should fail as well, namely:

protocol P1: Differentiable {
  @differentiable(wrt: x)
  func method(x: Float) -> Float
}

protocol P2: P1 {}
extension P2 {
  @differentiable(wrt: (self, x))
  func method(x: Float) -> Float { return 0 }
}

struct S: P2 {}

However this succeeds to compile today, because Sema is adding an implicit @differentiable attribute to the protocol extension method when checking the conformance. Now this will lead to very surprising behavior, consider the following example:

// File 1
protocol P1: Differentiable {
  @differentiable(wrt: x)
  func method(x: Float) -> Float
}

protocol P2: P1 {}
extension P2 {
  public func method(x: Float) -> Float { return 0 }
}

struct S1: P2 {} // Removing this will non-obviously cause file 2 to fail to compile

// File 2
struct S2: P2 {}

File 1 defines the S1: P2 conformance. It generates an implicit @differentiable(wrt: x) attribute on P2.method(x:) so that it is now part of the file's interface. File 2 compiles successfully as well, because when the compiler type-checks S2: P2 it finds a @differentiable(wrt: x) attribute.

Now, we modify file 1 by doing something that's seemingly completely harmless to file 2, that is, removing struct S1: P2 {}. Then file 2 will fail to compile! Because the implicit @differentiable(wrt: x) attribute is no longer generated. This is an API breakage that happened completely implicitly and is not at all obvious to the file 1's developer what broke it. I'm not sure but this issue may also happen cross-module, because type-checking S1: P2 is adding an implicit attribute that modifies P2.method(x:)'s public interface, causing even bigger API breakage issues.

To sum up, IMO the root problem is not at all related to whether conformances are declared in the same file as the witness candidate in a protocol extension, it is the fact that protocol conformance checking is modifying arbitrarily other declarations implicitly, causing their interface to change in an entirely non-obvious way. To solve this problem, for this example:

public protocol P1: Differentiable {
  @differentiable(wrt: x)
  func method(x: Float) -> Float
}

public protocol P2: P1 {}
extension P2 {
  public func method(x: Float) -> Float { return 0 }
}

struct S1: P2 {}

IMO the right behavior we should go with is to not emit any implicit @differentiable attributes on anything but candidates declared inside the conforming type. So the example above should fail to compile regardless of whether S1 is defined in the current file or not. When a candidate declaration doesn't have an attribute that satisfies the protocol requirement, it is not a valid candidate.

Case 2: Candidates in protocol extensions that effectively match

Now, back to the original example in your PR description:

import _Differentiation

protocol P1: Differentiable {
  @differentiable(wrt: self)
  func callAsFunction(_ input: Float) -> Float
}

protocol P2: P1 {}

extension P2 {
  @differentiable(wrt: (self, input))
  public func callAsFunction(_ input: Float) -> Float {
    return input
  }
}

struct S: P2 {}

Semantically, P2.callAsFunction(_:) declared in the protocol extension effectively matches the protocol requirement, so ideally this case should be supported without errors. When type checking S: P2, maybe we can just generate an implicit function declaration in S whose body calls the protocol extension method. This will then inherit the @differentiable attribute and AD will differentiate it correctly just like how we generate a subset parameters thunk. This should be regardless of whether the conformance is in the same file or not, once we've resolved Case 1.

Case 2 is nontrivial to implement but we need to fix case 1 to avoid those semantic issues in cross-file and cross-module cases.

To summarize, today's protocol conformance checking is adding implicit attributes to other declarations. IMO this attribute-adding behavior should be limited, in a way that is more limiting than what you've done for this patch, to only declarations declared in the conforming type's declaration context. Does that make sense?

dan-zheng · 2020-09-03T04:24:57Z

Thanks for the detailed write-up!

IMO this attribute-adding behavior should be limited, in a way that is more limiting than what you've done for this patch, to only declarations declared in the conforming type's declaration context. Does that make sense?

I think this makes sense, and I'll look into it.

Edit: it actually matters whether the witness declaration is in the same file as the conformance. Here's a simple example:

// main.swift

// Conformance declaration here.
extension S: P {}

// other_file.swift
import _Differentiation

protocol P: Differentiable {
  @differentiable(wrt: (self, x))
  func method(_ x: Float) -> Float
}

struct S: Differentiable {
  // Witness declaration here.
  func method(_ x: Float) -> Float { x }
}

$ swiftc main.swift other_file.swift
Undefined symbols for architecture x86_64:
  "_AD__$s4main1SV6methodyS2fF_PSSRS", referenced from:
      _AD__$s4main1SVAA1PA2aDP6methodyS2fFTW_jvp_SS in main-a5970c.o
      _AD__$s4main1SVAA1PA2aDP6methodyS2fFTW_vjp_SS in main-a5970c.o
ld: symbol(s) not found for architecture x86_64

This is because compiling main.swift (with the conformance) triggers implicit @differentiable attribute creation in other_file.swift (with the witness declaration). Derivative function protocol witness thunks created for main.swift reference derivative function symbols in other_file.swift that do not actually exist.

To get the desired semantics, I'll limit implicit @differentiable attribute creation to occur only when the conformance and witness declaration are (1) in the same type context and (2) in the same file.

New error message:

$ swiftc main.swift other_file.swift
main.swift:4:1: error: type 'S' does not conform to protocol 'P'
extension S: P {}
^
other_file.swift:11:8: note: candidate is missing explicit '@differentiable' attribute to satisfy requirement 'method' (in protocol 'P'); explicit attribute is necessary because candidate is declared in a different type context or file than the conformance of 'S' to 'P'
  func method(_ x: Float) -> Float { x }
       ^
  @differentiable
other_file.swift:6:8: note: protocol requires function 'method' with type '(Float) -> Float'
  func method(_ x: Float) -> Float
       ^

During protocol witness matching for a protocol requirement with `@differentiable` attributes, implicit `@differentiable` attributes may be created for the witness under specific conditions (when the witness has a `@differentiable` attribute with superset differentiability parameters, or when the witness has less-than-public visibility). Do not generate implicit `@differentiable` attributes for protocol witnesses when the protocol conformance is declared from a separate file from the witness. Otherwise, compilation of the file containing the conformance creates references to external symbols for the implicit `@differentiable` attributes, even though no such symbols exist. Resolves SR-13455.

Change phrasing to remove "blame" from the non-matching witness. Remove redundant DescriptiveDeclKind arguments.

…col witnesses. Require explicit `@differentiable` attribute on witnesses declared in a different type context or file than the protocol conformance.

dan-zheng · 2020-09-19T08:26:00Z

@swift-ci Please test

* 'master' of github.com:apple/swift: [AutoDiff] [Sema] Limit implicit `@differentiable` attribute creation. (swiftlang#33776)

swiftlang#33776) During protocol witness matching for a protocol requirement with `@differentiable` attributes, implicit `@differentiable` attributes may be created for the witness under specific conditions (when the witness has a `@differentiable` attribute with superset differentiability parameters, or when the witness has less-than-public visibility). Do not generate implicit `@differentiable` attributes for protocol witnesses when the protocol conformance is declared from a separate file or type context from the witness. Resolves SR-13455.

rxwei · 2020-10-15T20:06:12Z

This has broken some code where there are multiple @differentiable attributes on a protocol requirement. Implicit attributes should always be created when the declaration being type-checked has a differentiable attribute that covers a superset of differentiable parameters.

public protocol P: Differentiable {
    @differentiable(wrt: self)
    @differentiable(wrt: (self, x))
    func foo(_ x: Float) -> Float
}

// In a different file:
public struct S: P {
    // Should generate implicit `@differentiable(wrt: self)` here.
    @differentiable(wrt: (self, x))
    public func foo(_ x: Float) -> Float {
        x
    }
}

I'll fix this.

@differentiable

When checking protocol conformances with `@differentiable` requirements, the type checker is supposed to accept omissions of `@differentiable` attributes when there exsits an attribute that covers a superset of the differentiation configuration. This was accidentally regressed in swiftlang#33776 which made the following test case fail to compile. This is fixed by adjusting the witness matching conditions. ```swift // rdar://70348904 reproducer: public protocol P: Differentiable { @differentiable(wrt: self) @differentiable(wrt: (self, x)) func foo(_ x: Float) -> Float } public struct S: P {} extension S { // This had worked until swiftlang#33776. @differentiable(wrt: (self, x)) public func foo(_ x: Float) -> Float { x } } ``` Also fix some suboptimal diagnostics where more information could be shown. Resolves rdar://70348904.

@differentiable

…34533) When checking protocol conformances with `@differentiable` requirements, the type checker is supposed to accept omissions of `@differentiable` attributes when there exsits an attribute that covers a superset of the differentiation configuration. This was accidentally regressed in #33776 which made the following test case fail to compile. This is fixed by adjusting the witness matching conditions. ```swift // rdar://70348904 reproducer: public protocol P: Differentiable { @differentiable(wrt: self) @differentiable(wrt: (self, x)) func foo(_ x: Float) -> Float } public struct S: P {} extension S { // This had worked until #33776. @differentiable(wrt: (self, x)) public func foo(_ x: Float) -> Float { x } } ``` Also fix some suboptimal diagnostics where more information could be shown. Resolves rdar://70348904.

dan-zheng requested review from rxwei, texasmichelle and efremale September 3, 2020 00:05

texasmichelle approved these changes Sep 3, 2020

View reviewed changes

rxwei reviewed Sep 3, 2020

View reviewed changes

dan-zheng added 3 commits September 17, 2020 12:51

Improve "missing @differentiable attribute" diagnostic message.

f7ddfa7

Change phrasing to remove "blame" from the non-matching witness. Remove redundant DescriptiveDeclKind arguments.

Update requirements for explicit @differentiable attribute on proto…

a011d65

…col witnesses. Require explicit `@differentiable` attribute on witnesses declared in a different type context or file than the protocol conformance.

dan-zheng force-pushed the SR-13455 branch from ae3bf6c to a011d65 Compare September 19, 2020 05:14

dan-zheng requested a review from rxwei September 19, 2020 05:15

rxwei approved these changes Sep 19, 2020

View reviewed changes

dan-zheng merged commit 9afad73 into swiftlang:master Sep 19, 2020

dan-zheng deleted the SR-13455 branch September 19, 2020 17:49

rxwei mentioned this pull request Nov 1, 2020

[AutoDiff] [Sema] Fix '@differentiable' witness matching regression. #34533

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[AutoDiff] [Sema] Limit implicit `@differentiable` attribute creation. #33776

[AutoDiff] [Sema] Limit implicit `@differentiable` attribute creation. #33776

Uh oh!

dan-zheng commented Sep 3, 2020

Uh oh!

dan-zheng commented Sep 3, 2020

Uh oh!

texasmichelle left a comment

Uh oh!

rxwei left a comment

Uh oh!

rxwei commented Sep 3, 2020

Uh oh!

dan-zheng commented Sep 3, 2020 •

edited

Loading

Uh oh!

dan-zheng commented Sep 3, 2020 •

edited

Loading

Uh oh!

dan-zheng commented Sep 3, 2020

Uh oh!

rxwei commented Sep 3, 2020 •

edited

Loading

Uh oh!

dan-zheng commented Sep 3, 2020 •

edited

Loading

Uh oh!

dan-zheng commented Sep 19, 2020

Uh oh!

rxwei commented Oct 15, 2020

Uh oh!

Uh oh!

[AutoDiff] [Sema] Limit implicit @differentiable attribute creation. #33776

[AutoDiff] [Sema] Limit implicit @differentiable attribute creation. #33776

Uh oh!

Conversation

dan-zheng commented Sep 3, 2020

Uh oh!

dan-zheng commented Sep 3, 2020

Uh oh!

texasmichelle left a comment

Choose a reason for hiding this comment

Uh oh!

rxwei left a comment

Choose a reason for hiding this comment

Uh oh!

rxwei commented Sep 3, 2020

Uh oh!

dan-zheng commented Sep 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dan-zheng commented Sep 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dan-zheng commented Sep 3, 2020

Uh oh!

rxwei commented Sep 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Case 1: Candidates in protocol extensions that do not match

Case 2: Candidates in protocol extensions that effectively match

Uh oh!

dan-zheng commented Sep 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dan-zheng commented Sep 19, 2020

Uh oh!

rxwei commented Oct 15, 2020

Uh oh!

Uh oh!

[AutoDiff] [Sema] Limit implicit `@differentiable` attribute creation. #33776

[AutoDiff] [Sema] Limit implicit `@differentiable` attribute creation. #33776

dan-zheng commented Sep 3, 2020 •

edited

Loading

dan-zheng commented Sep 3, 2020 •

edited

Loading

rxwei commented Sep 3, 2020 •

edited

Loading

dan-zheng commented Sep 3, 2020 •

edited

Loading