-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Generic typealias fixes #3181
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Generic typealias fixes #3181
Conversation
@swift-ci Please test and merge |
@@ -0,0 +1,9 @@ | |||
// RUN: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you move this test to test/Serialization/ instead, and put the helper file in the Inputs directory? test/multifile/ is supposed to be for intra-module multi-file issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been abusing it lately to cover both intra- and inter-module issues.
Note that some of these tests were not specific to serialization, there were also SILGen issues. I'd rather keep them here as 'integration' tests for our multi-file support.
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't consider cross-module issues to be multi-file support. The code paths are almost completely different.
Maybe this is just an issue of terminology, but I don't think cross-module tests should be mixed in with intra-module tests, regardless of the directory name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The thing is a lot of the SILGen issues came up with both cross-file and cross-module things.
What about splitting up test/multifile then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think in this case the issue is clearly with serialization so the test should be moved there. But I'd like to keep the existing tests in multi-file as is. Sounds reasonable?
820aca4
to
6ddcf9a
Compare
@swift-ci Please test and merge |
// | ||
|
||
// FIXME: This should work? | ||
typealias OurType = MyType // expected-error {{reference to generic type 'MyType' requires arguments in <...>}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is right to reject this, we want a declaration to be obvious that it is generic, and state its type parameters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately the qualified lookup case does work, and at least xctest relies on it.
I'd be in favor of making this behavior consistent between the qualified and unqualified cases. It seems like there would be some fallout though.
Thank you for working on this Slava! |
…ging to a typealias The next patch creates a situation where we serialize a reference to a TypeAliasDecl's GenericParamTypeDecl, which references the inner DeclContext of the TypeAliasDecl itself. This was not being deserialized properly, triggering assertions.
…ature We were forgetting to do this, triggering crashes when using a generic typealias from another module. Fixes <https://bugs.swift.org/browse/SR-1889>.
The underlying type can now refer to generic parameters from an outer context, and we allow qualified and unqualified access to such typealiases. One problem remains, with specializations of generic typealiases in expression parsing context, marked with FIXME in the test.
…d protocols When we form calls to these methods, the ApplyExpr is wrapped in an additional conversion. Dig through these to produce the right diagnostic (or not, if the method is @discardableResult). Fixes <https://bugs.swift.org/browse/SR-1890>.
6ddcf9a
to
beb5018
Compare
@swift-ci Please test and merge |
Uh oh!
There was an error while loading. Please reload this page.