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

Jump to bottom

regression in case classes over java existential #8283

Closed
scabug opened this issue Feb 14, 2014 · 11 comments
Closed

regression in case classes over java existential #8283

scabug opened this issue Feb 14, 2014 · 11 comments
Assignees
@adriaanm
Labels
blocker has PR
Milestone
2.11.0-RC1

Comments

@scabug
Copy link

scabug commented Feb 14, 2014 

tail -n1000 test/files/pos/existential-java-fbound/{ProcessorDefinition,Client}.*
==> test/files/pos/existential-java-fbound/ProcessorDefinition.java <==
public abstract class ProcessorDefinition<Type extends ProcessorDefinition<Type>> {
}

==> test/files/pos/existential-java-fbound/Client.scala <==
case class ConsumerConfig(onRouteDefinition: Any => ProcessorDefinition[_])

// Workaround 1.
// Explicitly define the companion object.
// Otherwise, the synthetic companion extends `Function1` with a
// mismatch between the existential between the type arg and the apply signature.
//
// object ConsumerConfig

/*
<synthetic>
object ConsumerConfig
  extends scala.runtime.AbstractFunction1[Any => ProcessorDefinition[X] forSome { type X <: ProcessorDefinition[X] },ConsumerConfig]
     with Serializable {
  ...
  case <synthetic> def apply(onRouteDefinition: Any => ProcessorDefinition[_]): ConsumerConfig = new ConsumerConfig(onRouteDefinition);
  ...
}
*/

// Workaround 2: be explicit in the existential type, this will be accurately copied to the
// apply method
case class Okay(onRouteDefinition: Any => (ProcessorDefinition[X] forSome { type X <: ProcessorDefinition[X]}))
@scabug
Copy link
Author

scabug commented Feb 14, 2014

Imported From: https://issues.scala-lang.org/browse/SI-8283?orig=1
Reporter: @retronym

@scabug
Copy link
Author

scabug commented Feb 14, 2014

@retronym said (edited on Feb 14, 2014 11:11:24 AM UTC):
A simpler example (no f-bounds):

% tail -n1000 test/files/pos/existential-java-case-class/*
==> test/files/pos/existential-java-case-class/Client.scala <==
case class CC(x: J[_])

==> test/files/pos/existential-java-case-class/J.java <==
public class J<T extends String> {}

% qbin/scalac test/files/pos/existential-java-case-class/*
test/files/pos/existential-java-case-class/Client.scala:1: error: object creation impossible, since method apply in trait Function1 of type (v1: J[_])CC is not defined
(Note that T1 does not match J[_ <: String])
case class CC(x: J[_])
           ^
one error found

@scabug
Copy link
Author

scabug commented Feb 14, 2014

@retronym said:
The simplified example actually also fails in 2.10.3, albeit in a different spot, with:

test/files/pos/existential-java-case-class/Client.scala:1: error: type arguments [_$1] do not conform to class J's type parameter bounds [T <: J[String]]
case class CC(x: J[_]) {
           ^

@scabug
Copy link
Author

scabug commented Feb 14, 2014

@retronym said:
Okay, I think we need to rethink the mechanism in sharpenQuantifierBounds. The dangerous part about is is that it mutates the bounds of the quantified syms. So the sharper bounds aren't just used during subtyping; they leak back into the original ExistentialType. That's how they end up in the "derived" signatures of the case class synthetics.

I suspect we'll get less cross-talk if skolemizeExistential only sharpens the bounds of the skolems.

@scabug
Copy link
Author

scabug commented Feb 14, 2014

@retronym said:
Here's the sort of thing I have in mind: https://github.com/retronym/scala/compare/ticket;8283?expand=1

@scabug
Copy link
Author

scabug commented Feb 14, 2014

@adriaanm said:
thanks, your patch LGM -- shall I submit the PR?

@scabug
Copy link
Author

scabug commented Feb 14, 2014

@retronym said (edited on Feb 14, 2014 4:32:13 PM UTC):
Yep, that would be great. I still need to push #3452 over the line, and the clock is ticking :)

Depending on what sort of mood you are in, you could also consider -Yinfer-bounds to turn this on generally (rather than just for Java defined underlying types)

@scabug
Copy link
Author

scabug commented Feb 14, 2014

@adriaanm said:
will do!

@scabug
Copy link
Author

scabug commented Feb 14, 2014

@adriaanm said:
https://github.com/adriaanm/scala/compare/t8283?expand=1

@scabug
Copy link
Author

scabug commented Feb 14, 2014

@retronym said:
I just force pushed a few tiny cleanups.

diff --git a/src/reflect/scala/reflect/internal/Types.scala b/src/reflect/scala/reflect/internal/Types.scala
index be13082..53cf515 100644
--- a/src/reflect/scala/reflect/internal/Types.scala
+++ b/src/reflect/scala/reflect/internal/Types.scala
@@ -2571,7 +2571,10 @@ trait Types
       // (aka \Gamma, which tracks types for variables and constraints/kinds for types)
       // as a nice bonus, delaying this until we need it avoids cyclic errors
       val tpars = underlying.typeSymbol.initialize.typeParams
-      val rawToExistentialCreatedMe = (quantified corresponds underlying.typeArgs){ (q, a) => q.tpe =:= a }
+
+      // Is this existential of the form: T[Q1, ..., QN] forSome { type Q1 >: L1 <: U1, ..., QN >: LN <: UN}
+      val isExactApplication = (quantified corresponds underlying.typeArgs){ (q, a) => q.tpe =:= a }
+
       def newSkolem(basis: Symbol) = (owner orElse basis.owner).newExistentialSkolem(basis, origin)
       def newSharpenedSkolem(quant: Symbol, tparam: Symbol): Symbol = {
         def emptyBounds(sym: Symbol) = sym.info.bounds.isEmptyBounds
@@ -2590,7 +2593,7 @@ trait Types
             quant
         newSkolem(basis)
       }
-      if (rawToExistentialCreatedMe) deriveType2(quantified, tpars, newSharpenedSkolem)(underlying)
+      if (isExactApplication) deriveType2(quantified, tpars, newSharpenedSkolem)(underlying)
       else deriveType(quantified, newSkolem)(underlying)
     }

diff --git a/src/reflect/scala/reflect/internal/tpe/TypeMaps.scala b/src/reflect/scala/reflect/internal/tpe/TypeMaps.scala
index 351a0f3..07c9242 100644
--- a/src/reflect/scala/reflect/internal/tpe/TypeMaps.scala
+++ b/src/reflect/scala/reflect/internal/tpe/TypeMaps.scala
@@ -667,8 +667,6 @@ private[internal] trait TypeMaps {

   /** A base class to compute all substitutions */
   abstract class SubstMap[T](from: List[Symbol], to: List[T]) extends TypeMap {
-    if (!sameLength(from, to))
-      println("!!!")
     assert(sameLength(from, to), "Unsound substitution from "+ from +" to "+ to)

@scabug
Copy link
Author

scabug commented Feb 14, 2014

@adriaanm said:
scala/scala#3533

@scabug scabug closed this as completed Feb 14, 2014
@scabug scabug added this to the 2.11.0-RC1 milestone Apr 7, 2017
@scabug scabug mentioned this issue Apr 7, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@adriaanm adriaanm

Labels
blocker has PR
Projects
None yet
Milestone
2.11.0-RC1
Development

No branches or pull requests
2 participants
@adriaanm @scabug