Try to derive a type witness in a known conformance before attempting associated type inference #32578
Conversation
This file contains 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
|
@swift-ci Please test |
|
@swift-ci Please test source compatibility |
|
Build failed |
…which was always true
Sema can infer type witnesses for a small set of known conformances, to RawRepresentable, CaseIterable, and Differentiable. Previously, we would try to compute the type witness in this order: 1) First, via name lookup, to find an explicit nested type with the same name as an associated type. 2) Second, we would attempt inference. 3) Third, we would attempt derivation. Instead, let's do 3) before 2). This avoids circularity errors in situations where the witness can be derived, but inference fails. This breaks source compatibility with enum declarations where the raw type in the inheritance clause is a lie, and the user defines their own witnesses with mismatched types. However, I suspect this does not come up in practice, because if you don't synthesize witnesses, there is no way to access the actual raw literal values of the enum cases.
slavapestov
force-pushed
the
derive-type-witness-before-inference
branch
from
bf43ee1
to
dfbb958
Compare
|
@swift-ci Please test |
|
@swift-ci Please test source compatibility |
|
@swift-ci Please test |
|
@swift-ci Please test source compatibility |
|
@dan-zheng @rxwei Please take a look at the AutoDiff change I had to make here to get the test suite to pass. |
rxwei
approved these changes
Jun 26, 2020
|
The source compat failures are caused by a swiftpm issue and they're failing on the main branch as well. |
dan-zheng
added a commit
to tensorflow/swift-apis
that referenced
this pull request
Jul 1, 2020
Associated type inference behavior was changed in swiftlang/swift#32578: derived conformances are now attempted before associated type inference. This broke `ParameterlessLayer`, which relied on a `TangentVector == EmptyTangentVector` same-type constraint to set a default `TangentVector` type witness for conforming types. Add explicit `TangentVector` type witnesses to `ParameterlessLayer`-conforming types to fix this regression.
dan-zheng
added a commit
to tensorflow/swift-apis
that referenced
this pull request
Jul 2, 2020
Associated type inference behavior was changed in swiftlang/swift#32578: derived conformances are now attempted before associated type inference. This broke `ParameterlessLayer`, which relied on a `TangentVector == EmptyTangentVector` same-type constraint to set a default `TangentVector` type witness for conforming types. Add explicit `TangentVector` type witnesses to `ParameterlessLayer`-conforming types to fix this regression. This workaround is forward- and backward-compatible, but makes code more verbose.
dan-zheng
added a commit
to tensorflow/swift-models
that referenced
this pull request
Jul 3, 2020
Associated type inference behavior was changed in swiftlang/swift#32578: derived conformances are now attempted before associated type inference. This broke `ParameterlessLayer`, which relied on a `TangentVector == EmptyTangentVector` same-type constraint to set a default `TangentVector` type witness for conforming types. Add explicit `TangentVector` type witnesses to `ParameterlessLayer`-conforming types to fix this regression. This workaround is forward- and backward-compatible, but makes code more verbose.
dan-zheng
added a commit
to tensorflow/swift-models
that referenced
this pull request
Jul 3, 2020
Associated type inference behavior was changed in swiftlang/swift#32578: derived conformances are now attempted before associated type inference. This broke `ParameterlessLayer`, which relied on a `TangentVector == EmptyTangentVector` same-type constraint to set a default `TangentVector` type witness for conforming types. Add explicit `TangentVector` type witnesses to `ParameterlessLayer`-conforming types to fix this regression. This workaround is forward- and backward-compatible, but makes code more verbose.
This was referenced Jul 3, 2020
saeta
pushed a commit
to fastai/fastai_dev
that referenced
this pull request
Jul 3, 2020
This week, the S4TF team encountered several failures building toolchains: 1. Outdated Package.resolved in [swift/FastaiNotebook_11_imagenette](https://github.com/fastai/fastai_dev/blob/master/swift/FastaiNotebook_11_imagenette/Package.resolved). 2. A regression in `ParameterlessLayer` caused by a change in type inference behavior in [swiftlang/swift#32578](swiftlang/swift#32578). This PR includes two workarounds that unblock toolchain builds: 1. Remove Package.resolved ([swift-apis/1036](tensorflow/swift-apis#1036)). Since this file is regenerated on build, it's often not necessary to include in source control. 2. Add a typealias to instances conforming to `ParameterlessLayer`, which was effective in swift-apis and swift-models (see [tensorflow/swift-apis#1037](tensorflow/swift-apis#1037)). I'm happy to create an additional PR to remove the other Package.resolved files in this repo and add to .gitignore, if desired.
CodaFi
added a commit
to CodaFi/swift
that referenced
this pull request
Jul 29, 2020
A recent change to witness matching in swiftlang#32578 suddenly made the following construction illegal: // File1.swift enum MyEnumInAnotherFile { /**/ } // File2.swift extension MyEnumInAnotherFile { static var allCases: [MyEnumInAnotherFile] { /**/ } } Because it was no longer possible to derive the type witness for `AllCases`. This is because, when inference ran before synthesis, we would use the value witness to pick out the type witness and thus had no need for synthesis. Now that we run synthesis first, we ought to just allow deriving type witnesses across files, but still ban deriving value witnesses. In general, if you can utter a type in a different file to extend it, you should be able to see the requirements necessary to derive a default type witness. rdar://66279278, rdar://66279375, rdar://66279384, rdar://66279415, rdar://66279503
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.
Sema can infer type witnesses for a small set of known conformances, to RawRepresentable, CaseIterable, and Differentiable.
Previously, we would try to compute the type witness in this order:
First, via name lookup, to find an explicit nested type with the same name as an associated type.
Second, we would attempt inference.
Third, we would attempt derivation.
Instead, let's do 3) before 2). This avoids circularity errors in situations where the witness can be derived, but inference fails.
This breaks source compatibility with enum declarations where the raw type in the inheritance clause is a lie, and the user defines their own witnesses with mismatched types. However, I suspect this does not come up in practice, because if you don't synthesize witnesses, there is no way to access the actual raw literal values of the enum cases.