[AutoDiff] forbid derivative registration using @differentiable #30001
Conversation
|
@swift-ci please test |
There was a problem hiding this comment.
Yay!
When syntax definitions are changed, I believe apple/swift-syntax needs to be updated with newly generated syntax files. This is necessary to pass --swiftsyntax-verify-generated-files.
Here's an example. I think you can regenerate swift-syntax files with utils/build-script --llbuild --swiftpm --swiftsyntax.
|
Build failed |
|
Build failed |
|
For the swift-syntax changes, do I need to simultaneously merge this PR with the swift-syntax update? Is there some way I can run CI on the two PRs together so that I can see if they work together? @rintaro? If I do need to do it atomically, I'll split the swift-syntax changes out of this PR and do it separately so that I don't need to do the simultaneous merge thing with such a massive PR. |
Yes, you can run CI on the two PRs together. |
marcrasi
force-pushed
the
remove-differentiable-jvp-vjp
branch
from
47b7543 to
b9feb3d
Compare
|
Ok, I have rebased this and removed the syntax changes (will make them in a subsequent PR after merging this one). Running tests now. |
|
@swift-ci please test |
marcrasi
force-pushed
the
remove-differentiable-jvp-vjp
branch
from
b9feb3d to
1d2faf6
Compare
|
Rebased again. We're actually ready to merge this now. Running tests one more time. |
|
@swift-ci please test |
|
Merging to unblock progress (on TF-836 |
swiftlang/swift#30001 removed the logic from the parser to handle the `jvp:` and `vjp:` arguments of the attribute (but left the syntax definitions in place for the time being). Since this causes an attribute with those arguments to fail to parse, I've removed them from the tests so that those tests continue to pass under the new behavior. (The arguments were always optional, so they pass under the old behavior as well.) This also caught and fixed a bug where an attribute with only a `wrt:` and a `where` clause wasn't getting formatted correctly.
swiftlang/swift#30001 removed the logic from the parser to handle the `jvp:` and `vjp:` arguments of the attribute (but left the syntax definitions in place for the time being). Since this causes an attribute with those arguments to fail to parse, I've removed them from the tests so that those tests continue to pass under the new behavior. (The arguments were always optional, so they pass under the old behavior as well.) This also caught and fixed a bug where an attribute with only a `wrt:` and a `where` clause wasn't getting formatted correctly.
Forbids the
jvp:andvjp:arguments to@differentiable, deletes all the logic that handles them, and updates tests to use@derivative(of:)instead.I left an error message saying that
jvp:andvjp:are deprecated, so that there is a good explanation for anyone who missed the deprecation warning and whose code stops working. I think we should wait until next release to remove this.I have preemptively merged this into
tensorflowbecause it causes quite a few problems there (more tests need to be updated). PR for that coming soon.