[AutoDiff upstream] @derivative attribute type-checking fixes.
#28853
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rxwei
approved these changes
Dec 18, 2019
|
Could you also include the tests from #28785, which I just remembered to merge? |
dan-zheng
commented
Dec 18, 2019
| // before extracting the differential/pullback type from it, so that the | ||
| // derivative interface type generic signature is available for simplifying | ||
| // types. | ||
| CanType canActualResultType = derivativeInterfaceType->getCanonicalType(); |
There was a problem hiding this comment.
I wonder if it's possible to canonicalize derivativeInterfaceType earlier, like:
- auto *derivativeInterfaceType = derivative->getInterfaceType()
- ->castTo<AnyFunctionType>();
+ auto derivativeInterfaceType = dyn_cast<AnyFunctionType>(
+ derivative->getInterfaceType()->getCanonicalType());I tried this on tensorflow branch but it didn't work out of the box:
/Users/danielzheng/swift-tf/swift/test/AutoDiff/derivative_attr_type_checking.swift:58:25: error: incorrect message found
Assertion failed: ((size_t)sys::locale::columnWidth(I->getText()) == I->getText().size()), function buildFixItLine, file /Users/danielzheng/swift-tf/llvm-project/llvm/lib/Support/SourceMgr.cpp, line 316.
I believe this error occurs when diagnostic messages contain archetypes (τ_0_0). Filed TF-1073 to track this improvement.
| })) | ||
| return false; | ||
|
|
||
| // If required result type is not a function type, check that result types | ||
| // match exactly. | ||
| auto requiredResultFnTy = dyn_cast<AnyFunctionType>(required.getResult()); | ||
| auto candidateResultTy = mapType(candidateFnTy.getResult()); |
There was a problem hiding this comment.
I wonder if it's possible to remap candidateFnTy once, instead of remapping its individual components.
Filed TF-1073 to track this improvement.
Upstream `@derivative` attribute type-checking fixes regarding derivative generic signatures with all concrete generic parameters. Cherry-picked from: - swiftlang#28762 - swiftlang#28772
dan-zheng
force-pushed
the
derivative-attr-type-checking
branch
from
7fd79ed to
1d66571
Compare
|
@swift-ci Please test and merge |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Upstream
@derivativeattribute type-checking fixes regarding derivativegeneric signatures with all concrete generic parameters.
Cherry-picked from: