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

compiler infers non-wildcard existential type for case-module-unapply #6541

Closed
scabug opened this issue Oct 19, 2012 · 15 comments
Closed

compiler infers non-wildcard existential type for case-module-unapply #6541

scabug opened this issue Oct 19, 2012 · 15 comments
Assignees
Milestone

Comments

@scabug
Copy link

scabug commented Oct 19, 2012

trait A
trait B[T]
case class C(a: A, b: B[_])

generates the following unapply method

case <synthetic> def unapply(x$0: C): Option[(A, B[_$1])] forSome { type _$1 } =
  if (x$0.==(null))
    scala.this.None
  else
    Some.apply[(A, B[_$1])](Tuple2.apply[A, B[_$1]](x$0.a, x$0.b));

The problematic return type

Option[(A, B[_$1])] forSome { type _$1 }

is not generated together with the unapply method, but inferred by the compiler (when the generated method is type-checked). Martin suggests to provide the return type explicitly.

Also, in the body of the unapply, what is the type name _$1 is bound to?

@scabug
Copy link
Author

scabug commented Oct 19, 2012

Imported From: https://issues.scala-lang.org/browse/SI-6541?orig=1
Reporter: @lrytz
Affected Versions: 2.11.0-M1

@scabug
Copy link
Author

scabug commented Oct 19, 2012

@szeiger said:
To clarify, the correct type for the extractor should be

  Option[(A, B[_$1 forSome { type _$1 }])]

aka

  Option[(A, B[_])]

right?

@scabug
Copy link
Author

scabug commented Oct 19, 2012

@Blaisorblade said:
I think that Option[(A, B[])] is right, but that's defined by the SLS (3.2.10) to be equivalent to Option[(A, B[X] forSome {type X})], although that was counterintuitive also for me.
That makes a different when B is contravariant, since Option[(A, B[X] forSome {type X})] is then equivalent to Option[(A, B[Nothing])] while Option[(A, B[
$1 forSome { type _$1 }])] is equivalent to Option[(A, B[Any])] (if I get simplificaton rule 4 in SLS 3.2.10 right).

@scabug
Copy link
Author

scabug commented Oct 23, 2012

@adriaanm said:
not a regression, so not up for fixing this during the RC cycle
affects binary compatibility, so can't do it in 2.10.x
ergo, 2.11.x

@scabug
Copy link
Author

scabug commented Dec 11, 2012

@Blaisorblade said:
To clarify yet again (since I'm confused), the compiler should in fact infer: Option[(A, B[X] forSome {type X})], that is, Option[(A, B[_])].

It seems that in the body of the method, _$1 should be bound by matching on B's type: x$0.b match {case b: B[t] => Some((x$0.a, b))}. But maybe _$1 is in fact bound in some equivalent way, which can only be seen in the AST (trying to take a look now). Alternatively, the AST is just inconsistent and this is not detected, but that doesn't sound likely.

But I don't get what the impact of the bug is from the bug report. Looking at the thread, I see that:

  1. We get a warning on generated code.
  2. The generated type is wrong, and that comes from type inference, so it's somewhat confusing - is there a type inference bug?
  3. The generated code might be wrong, and maybe the error is swallowed later by erasure.
    We don't know if the generated code works, but I'd guess so.

@scabug
Copy link
Author

scabug commented Jan 9, 2013

@retronym said:
Here's a sketch of how we could address this. It's surprising tricky to get at all the information do to this during unapply synthesis (at least, it seems that way to me.)

https://github.com/retronym/scala/compare/ticket/6541

@scabug
Copy link
Author

scabug commented Apr 23, 2013

@paulp said:
I hadn't realized how easy it is to hit this:

scala> case class Foo(clazz: Class[_])
<console>:7: warning: inferred existential type Option[Class[_$1]] forSome { type _$1 }, which cannot be expressed by wildcards,  should be enabled
by making the implicit value scala.language.existentials visible.
This can be achieved by adding the import clause 'import scala.language.existentials'
or by setting the compiler option -language:existentials.
See the Scala docs for value scala.language.existentials for a discussion
why the feature should be explicitly enabled.
       case class Foo(clazz: Class[_])
                  ^
defined class Foo

@scabug
Copy link
Author

scabug commented Oct 15, 2013

@gkossakowski said:
Unassigning and rescheduling to M7 as previous deadline was missed.

@scabug
Copy link
Author

scabug commented Oct 1, 2014

@retronym said:
Batter up! scala/scala#4017

@scabug
Copy link
Author

scabug commented Nov 1, 2016

Eyal Roth (errr) said (edited on Nov 1, 2016 2:08:39 PM UTC):
Scala 2.11.8 yields the inferred existential type warning yet again because of this.

@scabug
Copy link
Author

scabug commented Nov 7, 2016

@SethTisue said:
I didn't look too deeply, but I think this is fixed in 2.12.0? Paul's example doesn't warn, nor does Lukas's original code.

@scabug
Copy link
Author

scabug commented Nov 7, 2016

@lrytz said:
yeah, forgot to close this: scala/scala#4017

@scabug
Copy link
Author

scabug commented Nov 7, 2016

@lrytz said:
oh wait, i see now it was reopened recently?

@scabug
Copy link
Author

scabug commented Nov 7, 2016

@lrytz said:
i will take a look.

@scabug
Copy link
Author

scabug commented Nov 8, 2016

@lrytz said:
The fix works as expected, since 2.11.5. Note that in 2.11, -Xsource:2.12 is required.

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

No branches or pull requests

2 participants