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

Comments

@scabug
Copy link

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

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Author

commented Oct 15, 2013

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

@scabug

This comment has been minimized.

Copy link
Author

commented Oct 1, 2014

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

@scabug

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Author

commented Nov 7, 2016

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

@scabug

This comment has been minimized.

Copy link
Author

commented Nov 7, 2016

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

@scabug

This comment has been minimized.

Copy link
Author

commented Nov 7, 2016

@lrytz said:
i will take a look.

@scabug

This comment has been minimized.

Copy link
Author

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
Projects
None yet
2 participants
You can’t perform that action at this time.