SIL: Don't try to lower various illegal tuple types #18659
Merged
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
…n types Even with an opaque abstraction pattern, we must explode a parameter list containing __shared and __owned elements. Otherwise, we produce invalid lowered SIL types. Note that this is already an issue, because the stdlib has a handful of declarations using __owned.
1) It's possible to materialize a tuple value with an @escaping or @autoclosure element in it. I don't think this causes any bad behavior in 4.2 because these flags have no semantic effect after the type checker, but now I'm adding an assertion that will fire when such types are serialized, so let's make sure it doesn't happen by explicitly clearing out these flags when lowering tuples types. 2) It's also possible to materialize a tuple with a single vararg element. Again, this was not a problem in 4.2, but with the above change to start clearing tuple flags, we now end up in a situation where the lowered type is not a tuple, because TupleType::get() returns a ParenType if the tuple has one element that is not vararg (which it no longer is, because we just cleared all the flags). Fix the second problem by treating one-element vararg tuples just like tuples with inout, __shared and __owned elements, that is, by always exploding them when they appear at the top level of a function parameter list, ensuring we never try to materialize a value whose type is the entire tuple type. These problems all stem from the fact that lowering a function type with the opaque abstraction pattern treats the top level argument list as a single tuple argument. Once that is fixed, much of the above will simplify down to assertions.
|
@swift-ci Please test |
|
@swift-ci Please test source compatibility |
|
@jckarter so as you were saying, all of this will be much simpler once we stop imploding the entire parameter list in re-abstraction thunks, and also if we stripped labels off in tuple type lowering. But for now, I'm trying to centralize the invariants here and add some tests for the edge cases that came up. |
CodaFi
approved these changes
Aug 12, 2018
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.
I added some assertions to prevent us from serializing tuple types with parameter flags except when part of a function type. This turned up some cases where SIL was constructing illegal tuple types.
Fix these in a sort of hacky way so that I can land the other changes until we rework SIL type lowering to use the new function type representation.