Skip to content

Commit

Permalink
SI-7859 Value classes may wrap a non-public member
Browse files Browse the repository at this point in the history
We allow value class constructors to be non-public, so to be regular,
we should also allow the same for the param accessor.

This commit uses the 'makeNotPrivate' machinery to ensure that
the backend can generate the requisite unboxing calls.

This commit:

  - refactors the code that enforced the restrictions, improving
    a few error messages and positions. The remaining restrictions
    needed some rewording in light of this change.
  - allows value classes to have non-public, val parameters.
    private[this] / protected[this] are still disallowed as value
    classes don't have a concept of `this`, and because trying to
    accomdate then would complicate the implementation.

    This means that `class C(x: Int) extends AnyVal` is not allowed,
    the user still must write `private val x: Int` or `val x: Int`.
  - Outlaw `class C()()(val x: Int) extends AnyVal` to curtail any
    bugs that might lurk in such a formulation.

The tests:

  - Show that the privacy is respected in the typer phase, under
    joint and separate compilation. We don't want a repeat performance
    of SI-6601.
  - Show that code that needs compiler-generated unboxes works under
    both compilation scenarios
  - Checks that the remaining restrictions are enforced and well
    communicated.
  • Loading branch information
retronym committed Sep 29, 2013
1 parent 0aaf591 commit 4595ac6
Show file tree
Hide file tree
Showing 10 changed files with 147 additions and 37 deletions.
3 changes: 3 additions & 0 deletions src/compiler/scala/tools/nsc/transform/ExtensionMethods.scala
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,9 @@ abstract class ExtensionMethods extends Transform with TypingTransformers {
checkNonCyclic(currentOwner.pos, Set(), currentOwner) */
extensionDefs(currentOwner.companionModule) = new mutable.ListBuffer[Tree]
currentOwner.primaryConstructor.makeNotPrivate(NoSymbol)
// SI-7859 make param accessors accessible so the erasure can generate unbox operations.
val paramAccessors = currentOwner.info.decls.filter(sym => sym.isParamAccessor && sym.isMethod)
paramAccessors.foreach(_.makeNotPrivate(currentOwner))
super.transform(tree)
} else if (currentOwner.isStaticOwner) {
super.transform(tree)
Expand Down
30 changes: 19 additions & 11 deletions src/compiler/scala/tools/nsc/typechecker/Typers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1356,17 +1356,25 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
unit.error(clazz.pos, "value class may not be a "+
(if (clazz.owner.isTerm) "local class" else "member of another class"))
if (!clazz.isPrimitiveValueClass) {
clazz.info.decls.toList.filter(acc => acc.isMethod && acc.isParamAccessor) match {
case List(acc) =>
def isUnderlyingAcc(sym: Symbol) =
sym == acc || acc.hasAccessorFlag && sym == acc.accessed
if (acc.accessBoundary(clazz) != rootMirror.RootClass)
unit.error(acc.pos, "value class needs to have a publicly accessible val parameter")
else if (acc.tpe.typeSymbol.isDerivedValueClass)
unit.error(acc.pos, "value class may not wrap another user-defined value class")
checkEphemeral(clazz, body filterNot (stat => isUnderlyingAcc(stat.symbol)))
case x =>
unit.error(clazz.pos, "value class needs to have exactly one public val parameter")
clazz.primaryConstructor.paramss match {
case List(List(param)) =>
val decls = clazz.info.decls
val paramAccessor = clazz.constrParamAccessors.head
if (paramAccessor.isMutable)
unit.error(paramAccessor.pos, "value class parameter must not be a var")
val accessor = decls.toList.find(x => x.isMethod && x.accessedOrSelf == paramAccessor)
accessor match {
case None =>
unit.error(paramAccessor.pos, "value class parameter must be a val and not be private[this]")
case Some(acc) if acc.isProtectedLocal =>
unit.error(paramAccessor.pos, "value class parameter must not be protected[this]")
case Some(acc) =>
if (acc.tpe.typeSymbol.isDerivedValueClass)
unit.error(acc.pos, "value class may not wrap another user-defined value class")
checkEphemeral(clazz, body filterNot (stat => stat.symbol != null && stat.symbol.accessedOrSelf == paramAccessor))
}
case _ =>
unit.error(clazz.pos, "value class needs to have exactly one val parameter")
}
}

Expand Down
4 changes: 2 additions & 2 deletions test/files/neg/anyval-anyref-parent.check
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ trait Foo2 extends AnyVal // fail
anyval-anyref-parent.scala:5: error: Any does not have a constructor
class Bar1 extends Any // fail
^
anyval-anyref-parent.scala:6: error: value class needs to have exactly one public val parameter
anyval-anyref-parent.scala:6: error: value class parameter must be a val and not be private[this]
class Bar2(x: Int) extends AnyVal // fail
^
^
anyval-anyref-parent.scala:10: error: illegal inheritance; superclass Any
is not a subclass of the superclass Object
of the mixin trait Immutable
Expand Down
19 changes: 19 additions & 0 deletions test/files/neg/t7859.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
B_2.scala:6: error: not found: value x
new p1.A(x).x
^
B_2.scala:6: error: value x in class A cannot be accessed in p1.A
new p1.A(x).x
^
B_2.scala:7: error: not found: value x
new B(x).x
^
B_2.scala:7: error: value x is not a member of B
new B(x).x
^
B_2.scala:8: error: not found: value x
new C(x).x
^
B_2.scala:8: error: value x in class C cannot be accessed in C
new C(x).x
^
6 errors found
5 changes: 5 additions & 0 deletions test/files/neg/t7859/A_1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package p1 {
class A(private[p1] val x: Any) extends AnyVal
}
class B(private val x: Any) extends AnyVal

9 changes: 9 additions & 0 deletions test/files/neg/t7859/B_2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
class C(private val x: Any) extends AnyVal

// Checking that makeNotPrivate(paramAccessor) doesn't make this visible during typer.
// The output is identical with/without `extends AnyVal`.
object Test {
new p1.A(x).x
new B(x).x
new C(x).x
}
43 changes: 23 additions & 20 deletions test/files/neg/valueclasses.check
Original file line number Diff line number Diff line change
Expand Up @@ -4,40 +4,43 @@ trait T extends AnyVal // fail
valueclasses.scala:6: error: value class may not be a member of another class
class Bar(x: Int) extends AnyVal // fail
^
valueclasses.scala:6: error: value class parameter must be a val and not be private[this]
class Bar(x: Int) extends AnyVal // fail
^
valueclasses.scala:8: error: value class may not be a local class
class Baz(x: Int) extends AnyVal // fail
^
valueclasses.scala:12: error: value class needs to have exactly one public val parameter
valueclasses.scala:8: error: value class parameter must be a val and not be private[this]
class Baz(x: Int) extends AnyVal // fail
^
valueclasses.scala:12: error: value class needs to have exactly one val parameter
class V1 extends AnyVal // fail
^
valueclasses.scala:14: error: value class needs to have a publicly accessible val parameter
class V2(private[test] val x: Int) extends AnyVal // fail
^
valueclasses.scala:15: error: value class needs to have a publicly accessible val parameter
class V3(protected[test] val x: Int) extends AnyVal // fail
^
valueclasses.scala:16: error: value class needs to have a publicly accessible val parameter
class V4(protected val x: Int) extends AnyVal // fail
^
valueclasses.scala:17: error: value class needs to have a publicly accessible val parameter
class V5(private val x: Int) extends AnyVal // fail
^
valueclasses.scala:19: error: value class needs to have exactly one public val parameter
valueclasses.scala:19: error: value class needs to have exactly one val parameter
class V6(val x: Int, val y: String) extends AnyVal // fail
^
valueclasses.scala:20: error: field definition is not allowed in value class
valueclasses.scala:20: error: value class needs to have exactly one val parameter
class V7(val x: Int, private[this] val y: String) extends AnyVal // fail
^
valueclasses.scala:21: error: value class needs to have exactly one public val parameter
class V8(var x: Int) extends AnyVal // fail
^
valueclasses.scala:21: error: value class parameter must not be a var
class V8(var x: Int) extends AnyVal // fail
^
valueclasses.scala:24: error: field definition is not allowed in value class
val y = x // fail
^
valueclasses.scala:29: error: type parameter of value class may not be specialized
class V12[@specialized T, U](val x: (T, U)) extends AnyVal // fail
^
valueclasses.scala:31: error: value class needs to have exactly one public val parameter
valueclasses.scala:31: error: value class parameter must be a val and not be private[this]
class V13(x: Int) extends AnyVal // fail
^
valueclasses.scala:33: error: value class parameter must be a val and not be private[this]
class V14(private[this] val x: Int) extends AnyVal // fail
^
valueclasses.scala:34: error: value class parameter must not be protected[this]
class V15(protected[this] val x: Int) extends AnyVal // fail
^
valueclasses.scala:36: error: value class needs to have exactly one val parameter
class V16()(val a: Any) extends AnyVal // fail, was allowed 2.10.x
^
14 errors found
15 errors found
13 changes: 9 additions & 4 deletions test/files/neg/valueclasses.scala
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ class Foo {

class V1 extends AnyVal // fail

class V2(private[test] val x: Int) extends AnyVal // fail
class V3(protected[test] val x: Int) extends AnyVal // fail
class V4(protected val x: Int) extends AnyVal // fail
class V5(private val x: Int) extends AnyVal // fail
class V2(private[test] val x: Int) extends AnyVal // okay, wasn't allowed in 2.10.x
class V3(protected[test] val x: Int) extends AnyVal // okay, wasn't allowed in 2.10.x
class V4(protected val x: Int) extends AnyVal // okay, wasn't allowed in 2.10.x
class V5(private val x: Int) extends AnyVal // okay, wasn't allowed in 2.10.x

class V6(val x: Int, val y: String) extends AnyVal // fail
class V7(val x: Int, private[this] val y: String) extends AnyVal // fail
Expand All @@ -29,3 +29,8 @@ class V11[T](val x: List[T]) extends AnyVal // ok
class V12[@specialized T, U](val x: (T, U)) extends AnyVal // fail

class V13(x: Int) extends AnyVal // fail

class V14(private[this] val x: Int) extends AnyVal // fail
class V15(protected[this] val x: Int) extends AnyVal // fail

class V16()(val a: Any) extends AnyVal // fail, was allowed 2.10.x
11 changes: 11 additions & 0 deletions test/files/run/t7859/A_1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
class A(private val x: Int) extends AnyVal

object A {
val Const = new A(0)
}

class A1(protected val x: Int) extends AnyVal

package p {
class A2(private[p] val x: Int) extends AnyVal
}
47 changes: 47 additions & 0 deletions test/files/run/t7859/B_2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
class B private (private val b: Int) extends AnyVal
object B {
val Const = new B(0)
}

// These tests will require erasure to unbox the value class.
// We need to test under joint and separate compilation to check
// that the 'notPRIVATE' flag on the param accessor is pickled.
//
// See also SI-6601.
object Test {
def main(args: Array[String]) {
unboxA
unboxA1
unboxA2
unboxB
}

def unboxA {
val o: Some[A] = Some(A.Const)
val a = o.get
def id(a: A): A = a
id(a)
}

def unboxA1 {
val o: Some[A1] = Some(new A1(0))
val a = o.get
def id(a: A1): A1 = a
id(a)
}

def unboxA2 {
import p.A2
val o: Some[A2] = Some(new A2(0))
val a = o.get
def id(a: A2): A2 = a
id(a)
}

def unboxB {
val o: Some[B] = Some(B.Const)
val b = o.get
def id(b: B): B = b
id(b)
}
}

0 comments on commit 4595ac6

Please sign in to comment.