-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix #2732: Allow wildcards in SAM types (take 2) #4152
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
Conversation
Narrowing the type is not necessary to check for instantiability, and it prevents valid SAM types with wildcards from being used. This commit does not contain any positive testcase because SAM types with wildcards need another fix present in the next commit to work.
test performance please |
performance test scheduled: 1 job(s) in queue, 1 running. |
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/4152/ to see the changes. Benchmarks is based on merging with master (0c5e39d) |
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 was just puzzled by the comment. The code LGTM.
// | ||
// the single abstract method will have type: | ||
// | ||
// (x: Function[_ >: String, _ <: Int]#T): Function[_ >: String, _ <: Int]#R |
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.
Is that really its type? By what means was that computed? I would have thought that it's unsound to assume the argument type as written.
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.
That's the type I saw, it comes from tp.abstractTermMembers
above where tp
is an AppliedType, I guess this cannot happen in regular code because you would always have a TermRef at this point.
No description provided.