Skip to content
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

Fix #2732: Allow wildcards in SAM types (take 2) #4152

Merged
merged 3 commits into from
Mar 23, 2018

Conversation

smarter
Copy link
Member

@smarter smarter commented Mar 21, 2018

No description provided.

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.
@smarter
Copy link
Member Author

smarter commented Mar 21, 2018

test performance please

@smarter smarter requested a review from odersky March 21, 2018 16:16
@dottybot
Copy link
Member

performance test scheduled: 1 job(s) in queue, 1 running.

@dottybot
Copy link
Member

Performance test finished successfully:

Visit http://dotty-bench.epfl.ch/4152/ to see the changes.

Benchmarks is based on merging with master (0c5e39d)

Copy link
Contributor

@odersky odersky left a 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
Copy link
Contributor

@odersky odersky Mar 23, 2018

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.

Copy link
Member Author

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.

@smarter smarter merged commit e1a18da into scala:master Mar 23, 2018
@allanrenucci allanrenucci deleted the fix-2732-take-2 branch March 23, 2018 22:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants