From b941551529e40fc7d71cf25e1ad904ab7badd14c Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Wed, 22 May 2013 15:51:18 +0200 Subject: SI-6138 Centralize and refine detection of `getClass` calls `getClass` is special cased in the compiler; this is described in in the comments on `Definitions.Any_getClass`. Part of this happens in `Typer#stabilize`. This was trying to determine if an Ident or Select node was a call to `getClass` by merits of the name of the tree's symbol and by checking that the its type (if it was a MethodType or PolyType) had no parameters in the primary parameter list. Overloaded user defined `getClass` methods confused this check. In the enclosed test case, the tree `definitions.this.getClass` had an `OverloadedType`, and such types always report an empty list of `params`. This commit: - changes `stabilize` to use `isGetClass`, rather than the homebrew check - changes `isGetClass` to consider a `Set[Symbol]` containing all `getClass` variants. This moves some similar code from `Erasure` to `Definitions` - keeps a fast negative path in `isGetClass` based on the symbol's name --- src/reflect/scala/reflect/internal/Definitions.scala | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) (limited to 'src/reflect') diff --git a/src/reflect/scala/reflect/internal/Definitions.scala b/src/reflect/scala/reflect/internal/Definitions.scala index e5d9e54a16..629d82d254 100644 --- a/src/reflect/scala/reflect/internal/Definitions.scala +++ b/src/reflect/scala/reflect/internal/Definitions.scala @@ -110,8 +110,10 @@ trait Definitions extends api.StandardDefinitions { /** Is symbol a numeric value class? */ def isNumericValueClass(sym: Symbol) = ScalaNumericValueClasses contains sym - def isGetClass(sym: Symbol) = - (sym.name == nme.getClass_) && flattensToEmpty(sym.paramss) + def isGetClass(sym: Symbol) = ( + sym.name == nme.getClass_ // this condition is for performance only, this is called from `Typer#stabliize`. + && getClassMethods(sym) + ) lazy val UnitClass = valueClassSymbol(tpnme.Unit) lazy val ByteClass = valueClassSymbol(tpnme.Byte) @@ -805,6 +807,12 @@ trait Definitions extends api.StandardDefinitions { lazy val Any_isInstanceOf = newT1NullaryMethod(AnyClass, nme.isInstanceOf_, FINAL)(_ => booltype) lazy val Any_asInstanceOf = newT1NullaryMethod(AnyClass, nme.asInstanceOf_, FINAL)(_.typeConstructor) + lazy val primitiveGetClassMethods = Set[Symbol](Any_getClass, AnyVal_getClass) ++ ( + ScalaValueClasses map (_.tpe member nme.getClass_) + ) + + lazy val getClassMethods: Set[Symbol] = primitiveGetClassMethods + Object_getClass + // A type function from T => Class[U], used to determine the return // type of getClass calls. The returned type is: // -- cgit v1.2.3 From 403eadd0f10cf6e493b81913148b0b9065e80699 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Wed, 29 May 2013 23:44:21 +0200 Subject: SI-7517 Fix higher kinded type inference regression - Discovered in 2.10.2-RC1 - Ostensibly regressed in 7e52fb910b, which conceptually reverted part of 0cde930b so that (mutable) TypeVars don't use structural equality. - But, does *not* fail if 7e52fb910b is cherry-picked directly after 0cde930b, suggesting that it shone a light on a behaviour change in some other commit in between the two. - Indeed, the true regression came in https://github.com/scala/scala/commit/e5da30b843#L5L3176 - A targeted revert of e5da30b843 is undesirable, as we'd like SI-6846 to stay fixed What's happening here? In the enclosed test case, higher kinded type inference explores two possibilities: Composed.this.Split[A] K[[T]A[B[T]]] // `Split[A]` dealiased The difference in the flow of type inference can be seen from the diff below. Notice how now we no longer register `?K.addBound(Composed.this.Split)`, we instead only register `?K.addBound(K)` ```patch --- sandbox/old.log 2013-05-30 00:27:34.000000000 +0200 +++ sandbox/new.log 2013-05-30 00:28:28.000000000 +0200 @@ -1,55 +1,114 @@ ?K.unifyFull(Composed.this.Split[A]) ?K.unifySpecific(Composed.this.Split[A]) - ?K.addBound(Composed.this.Split) ?B.unifyFull(T) ?B.unifySpecific(T) `-> false ?B.unifyFull(Any) ?B.unifySpecific(Any) `-> false `-> false ?K.unifySpecific(L[[T]A[B[T]]]) - ?K.addBound(L) ?B.unifyFull(B[T]) ?B.unifySpecific(B[T]) ?B.addBound(B) `-> true ?B.unifyFull(B[T]) ?B.unifySpecific(B[T]) ?B.addBound(B) `-> true ?B.unifyFull(B[T]) ?B.unifySpecific(B[T]) ?B.addBound(B) `-> true ?B.unifyFull(B[T]) ?B.unifySpecific(B[T]) ?B.addBound(B) `-> true + ?K.addBound(L) `-> true ?K.unifyFull(Composed.this.Split[A]) ?K.unifySpecific(Composed.this.Split[A]) - ?K.addBound(Composed.this.Split) ?B.unifyFull(x) ?B.unifySpecific(x) `-> false `-> false ?K.unifySpecific(L[[T]A[B[T]]]) + ?B.unifyFull(B[T]) + ?B.unifySpecific(B[T]) + ?B.addBound(B) + `-> true + ?B.unifyFull(B[T]) + ?B.unifySpecific(B[T]) + ?B.addBound(B) + `-> true + ?B.unifyFull(B[T]) + ?B.unifySpecific(B[T]) + ?B.addBound(B) + `-> true + ?B.unifyFull(B[T]) + ?B.unifySpecific(B[T]) + ?B.addBound(B) + `-> true ?K.addBound(L) + `-> true +?K.unifyFull(Composed.this.Split[A]) + ?K.unifySpecific(Composed.this.Split[A]) + ?B.unifyFull(T) + ?B.unifySpecific(T) + `-> false + ?B.unifyFull(Any) + ?B.unifySpecific(Any) + `-> false + `-> false + ?K.unifySpecific(L[[T]A[B[T]]]) ?B.unifyFull(B[T]) ?B.unifySpecific(B[T]) ?B.addBound(B) `-> true ?B.unifyFull(B[T]) ?B.unifySpecific(B[T]) ?B.addBound(B) `-> true ?B.unifyFull(B[T]) ?B.unifySpecific(B[T]) ?B.addBound(B) `-> true ?B.unifyFull(B[T]) ?B.unifySpecific(B[T]) ?B.addBound(B) `-> true + ?K.addBound(L) + `-> true +?K.unifyFull(Composed.this.Split[A]) + ?K.unifySpecific(Composed.this.Split[A]) + ?B.unifyFull(x) + ?B.unifySpecific(x) + `-> false + `-> false + ?K.unifySpecific(L[[T]A[B[T]]]) + ?B.unifyFull(B[T]) + ?B.unifySpecific(B[T]) + ?B.addBound(B) + `-> true + ?B.unifyFull(B[T]) + ?B.unifySpecific(B[T]) + ?B.addBound(B) + `-> true + ?B.unifyFull(B[T]) + ?B.unifySpecific(B[T]) + ?B.addBound(B) + `-> true + ?B.unifyFull(B[T]) + ?B.unifySpecific(B[T]) + ?B.addBound(B) + `-> true + ?K.addBound(L) + `-> true +?K.unifyFull(L[A]) + ?K.unifySpecific(L[A]) + ?K.addBound(L) + `-> true +?K.unifyFull(L[A]) + ?K.unifySpecific(L[A]) + ?K.addBound(L) `-> true ``` --- src/reflect/scala/reflect/internal/Types.scala | 7 +++---- test/files/pos/t7517.scala | 22 ++++++++++++++++++++++ 2 files changed, 25 insertions(+), 4 deletions(-) create mode 100644 test/files/pos/t7517.scala (limited to 'src/reflect') diff --git a/src/reflect/scala/reflect/internal/Types.scala b/src/reflect/scala/reflect/internal/Types.scala index ee584bed2c..0cc2b91a44 100644 --- a/src/reflect/scala/reflect/internal/Types.scala +++ b/src/reflect/scala/reflect/internal/Types.scala @@ -3218,10 +3218,9 @@ trait Types extends api.Types { self: SymbolTable => sameLength(typeArgs, tp.typeArgs) && { val lhs = if (isLowerBound) tp.typeArgs else typeArgs val rhs = if (isLowerBound) typeArgs else tp.typeArgs - // this is a higher-kinded type var with same arity as tp. - // side effect: adds the type constructor itself as a bound - addBound(tp.typeConstructor) - isSubArgs(lhs, rhs, params, AnyDepth) + // This is a higher-kinded type var with same arity as tp. + // If so (see SI-7517), side effect: adds the type constructor itself as a bound. + isSubArgs(lhs, rhs, params, AnyDepth) && { addBound(tp.typeConstructor); true } } } // The type with which we can successfully unify can be hidden diff --git a/test/files/pos/t7517.scala b/test/files/pos/t7517.scala new file mode 100644 index 0000000000..7ce4c6b13e --- /dev/null +++ b/test/files/pos/t7517.scala @@ -0,0 +1,22 @@ +trait Box[ K[A[x]] ] + +object Box { + // type constructor composition + sealed trait ∙[A[_], B[_]] { type l[T] = A[B[T]] } + + // composes type constructors inside K + type SplitBox[K[A[x]], B[x]] = Box[ ({ type l[A[x]] = K[ (A ∙ B)#l] })#l ] + + def split[ K[A[x]], B[x] ](base: Box[K]): SplitBox[K,B] = ??? + + class Composed[B[_], L[A[x]] ] { + val box: Box[L] = ??? + + type Split[ A[x] ] = L[ (A ∙ B)#l ] + val a: Box[Split] = Box.split(box) + + //Either of these work: + val a1: Box[Split] = Box.split[L,B](box) + val a2: Box[ ({ type l[A[x]] = L[ (A ∙ B)#l ] })#l ] = Box.split(box) + } +} \ No newline at end of file -- cgit v1.2.3 From d2faeb9ae60389668f1b5f45eb91c73127401e40 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Thu, 23 May 2013 09:25:11 +0200 Subject: SI-7507 Fix lookup of private[this] member in presence of self type. In the following code: trait Cake extends Slice trait Slice { self: Cake => // must have self type that extends `Slice` private[this] val bippy = () // must be private[this] locally(bippy) } `ThisType()`.findMember(bippy)` excluded the private local member on the grounds that the first class in the base type sequence, `Cake`, was not contained in `Slice`. scala> val thisType = typeOf[Slice].typeSymbol.thisType thisType: $r.intp.global.Type = Slice.this.type scala> thisType.baseClasses res6: List[$r.intp.global.Symbol] = List(trait Cake, trait Slice, class Object, class Any) This commit changes `findMember` to use the symbol of the `ThisType`, rather than the first base class, as the location of the selection. --- src/reflect/scala/reflect/internal/Types.scala | 12 ++++++++-- test/files/neg/t7507.check | 4 ++++ test/files/neg/t7507.scala | 7 ++++++ test/files/run/t7507.scala | 31 ++++++++++++++++++++++++++ 4 files changed, 52 insertions(+), 2 deletions(-) create mode 100644 test/files/neg/t7507.check create mode 100644 test/files/neg/t7507.scala create mode 100644 test/files/run/t7507.scala (limited to 'src/reflect') diff --git a/src/reflect/scala/reflect/internal/Types.scala b/src/reflect/scala/reflect/internal/Types.scala index ee584bed2c..fc3f5de77f 100644 --- a/src/reflect/scala/reflect/internal/Types.scala +++ b/src/reflect/scala/reflect/internal/Types.scala @@ -1172,6 +1172,14 @@ trait Types extends api.Types { self: SymbolTable => continue = false val bcs0 = baseClasses var bcs = bcs0 + // omit PRIVATE LOCALS unless selector class is contained in class owning the def. + def admitPrivateLocal(owner: Symbol): Boolean = { + val selectorClass = this match { + case tt: ThisType => tt.sym // SI-7507 the first base class is not necessarily the selector class. + case _ => bcs0.head + } + selectorClass.hasTransOwner(owner) + } while (!bcs.isEmpty) { val decls = bcs.head.info.decls var entry = decls.lookupEntry(name) @@ -1181,10 +1189,10 @@ trait Types extends api.Types { self: SymbolTable => if ((flags & required) == required) { val excl = flags & excluded if (excl == 0L && - (// omit PRIVATE LOCALS unless selector class is contained in class owning the def. + ( (bcs eq bcs0) || (flags & PrivateLocal) != PrivateLocal || - (bcs0.head.hasTransOwner(bcs.head)))) { + admitPrivateLocal(bcs.head))) { if (name.isTypeName || stableOnly && sym.isStable) { if (Statistics.canEnable) Statistics.popTimer(typeOpsStack, start) if (suspension ne null) suspension foreach (_.suspended = false) diff --git a/test/files/neg/t7507.check b/test/files/neg/t7507.check new file mode 100644 index 0000000000..d402869fd4 --- /dev/null +++ b/test/files/neg/t7507.check @@ -0,0 +1,4 @@ +t7507.scala:6: error: value bippy in trait Cake cannot be accessed in Cake + locally(bippy) + ^ +one error found diff --git a/test/files/neg/t7507.scala b/test/files/neg/t7507.scala new file mode 100644 index 0000000000..1b4756d955 --- /dev/null +++ b/test/files/neg/t7507.scala @@ -0,0 +1,7 @@ +trait Cake extends Slice { + private[this] val bippy = () +} + +trait Slice { self: Cake => + locally(bippy) +} diff --git a/test/files/run/t7507.scala b/test/files/run/t7507.scala new file mode 100644 index 0000000000..6c1959ddac --- /dev/null +++ b/test/files/run/t7507.scala @@ -0,0 +1,31 @@ +trait Cake extends Slice + +// Minimization +trait Slice { self: Cake => // must have self type that extends `Slice` + private[this] val bippy = () // must be private[this] + locally(bippy) +} + +// Originally reported bug: +trait Cake1 extends Slice1 +trait Slice1 { self: Cake1 => + import java.lang.String // any import will do! + val Tuple2(x, y) = ((1, 2)) +} + + +// Nesting +trait Cake3 extends Outer.Slice3 + +// Minimization +object Outer { + private[this] val bippy = () + trait Slice3 { self: Cake3 => + locally(bippy) + } +} + +object Test extends App { + val s1 = new Cake1 {} + assert((s1.x, s1.y) == (1, 2), (s1.x, s1.y)) +} -- cgit v1.2.3