Skip to content

Conversation

@odersky
Copy link
Contributor

@odersky odersky commented Oct 13, 2016

Fixes #1590. Type variables should never be instantiated to types
containing wildcards.

Review by @smarter

Fixes scala#1590. Type variables should never be instantiated to types
containing wildcards.
* of the parameter in a refinement somewhere in the bound are removed.
* of the parameter in a refinement somewhere in the bound are removed. Also
* wildcard types in bounds are approximated by their upper or lower bounds.
* (Such occurrences can arise for F-bounded types).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be moved before the sentence you added

@smarter
Copy link
Member

smarter commented Oct 13, 2016

With this fix, we end up with the equivalent of:

case class W[T](seq: Option[Option[T]] = Option.empty[Option[Any]])

But what we really want is:

case class W[T](seq: Option[Option[T]] = Option.empty[Option[T]])

The difference is apparent once you try to use the class:

case class W[T](seq: Option[Option[T]] = Option.empty)

object Test {
  W[Int]()
}

This compiles with scalac but with this patch we get:

-- [E007] Type Mismatch Error: tests/pos/i1590.scala -------------------------------------------------------------------
4 |  W[Int]()
  |  ^^^^^^^^
  |  found:    Option[Option[Any]]
  |  required: Option[Option[Int]]
  |  

@odersky
Copy link
Contributor Author

odersky commented Oct 13, 2016

We now get Option[Option[Nothing]] which is what scalac does as well. I remember there was a long discussion on JIRA somewhere why we cannot do better. The gist is that the default argument is not required to conform to the parameter type, so constraining it to be a subtype is wrong.

// (i.e. the whole bounds range is over the type)
// if variance > 0, pick the minimal safe type: bounds.hi
// (i.e. the whole bounds range is under the type)
// if variance == 0, pick bounds.lo anyway (this is arbitrary but in line with
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you mean bounds.hi here, or the code is wrong, in any case I'd love to see some tests for all these cases :).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah nevermind, I misread the condition.

@odersky odersky merged commit 0fdd4e3 into scala:master Oct 16, 2016
@allanrenucci allanrenucci deleted the fix-#1590 branch December 14, 2017 16:59
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.

2 participants