Skip to content

Fix #4496: call ensureAccessible to bypass class-level accessibility #4529

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

Merged
merged 1 commit into from
May 28, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions library/src/scala/reflect/Selectable.scala
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ class Selectable(val receiver: Any) extends AnyVal with scala.Selectable {
val rcls = receiver.getClass
try {
val fld = rcls.getField(name)
ensureAccessible(fld)
fld.get(receiver)
}
catch {
Expand All @@ -17,6 +18,7 @@ class Selectable(val receiver: Any) extends AnyVal with scala.Selectable {
val rcls = receiver.getClass
val paramClasses = paramTypes.map(_.runtimeClass)
val mth = rcls.getMethod(name, paramClasses: _*)
ensureAccessible(mth)
paramTypes.length match {
case 0 => () =>
mth.invoke(receiver)
Expand Down
25 changes: 25 additions & 0 deletions tests/neg/i4496b.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import scala.reflect.Selectable.reflectiveSelectable

trait Foo1 { val a: Int }
trait Foo2 { def a: Int }
trait Foo3 { var a: Int }

object TestStructuralVar {
type T0 = {var a: Int} // error
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated, but maybe this should be allowed and given the same semantics as having a getter/setter ?

Copy link
Member

@smarter smarter May 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On second thoughts, maybe that's not worth it, since then it needs to be meaningful for all possible interpretations of structural types, nevermind.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...but maybe the error message could suggest replacing the var by a val and a setter.

object TestStructuralVar {
type T = {val a: Int; def a_=(x: Int): Unit}
def upcast1(v: Foo1): T = v // error
def upcast2(v: Foo2): T = v // error
def upcast3(v: Foo3): T = v
def verify(v: T) = ()
def test(): Unit = {
verify(upcast1(new Foo1 { val a = 10 }))
verify(upcast2(new Foo2 { val a = 10 }))
verify(upcast3(new Foo3 { var a = 10 }))
}
}

def main(args: Array[String]): Unit = {
TestStructuralVar.test()
}
}
116 changes: 116 additions & 0 deletions tests/run/i4496b.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
import scala.reflect.Selectable.reflectiveSelectable

trait Foo1 { val a: Int }
trait Foo2 { def a: Int }
trait Foo3 { var a: Int }

object Test {
private class FooBar1 extends Foo1 { val a: Int = 10 }
private class FooBar2 extends Foo2 { def a: Int = 10 }
private class FooBar3 extends Foo3 { var a: Int = 10 }

private class Bar1 { val a: Int = 10 }
private class Bar2 { def a: Int = 10 }
private class Bar3 { var a: Int = 10 }

object TestStructuralVal {
// This test is also an (abstracted) motivating example.

// Consider one module upcasting all these instances to T. These casts are clearly well-typed.
type T = {val a: Int}
def upcast1(v: Foo1): T = v
def upcast2(v: Foo2): T = v
def upcast3(v: Foo3): T = v

// These accesses are also clearly well-typed
def consume(v: T) = v.a
inline def consumeInl(v: T) = v.a
def verify(v: T) = {
assert(consume(v) == 10)
assert(consumeInl(v) == 10)
assert(v.a == 10)
}

def test(): Unit = {
// These calls are also clearly well-typed, hence can't be rejected.
verify(upcast1(new Foo1 { val a = 10 }))
verify(upcast2(new Foo2 { val a = 10 }))
verify(upcast3(new Foo3 { var a = 10 }))
// Ditto, so we must override access control to the class.
verify(upcast1(new FooBar1))
verify(upcast2(new FooBar2))
verify(upcast3(new FooBar3))

// Other testcases
verify(new {val a = 10} : T)
verify(new {var a = 10} : T)
verify(new {def a = 10} : T)

verify(new Bar1 : T)
verify(new Bar2 : T)
verify(new Bar3 : T)
}
}

object TestStructuralDef {
type T = {def a: Int}
def upcast1(v: Foo1): T = v
def upcast2(v: Foo2): T = v
def upcast3(v: Foo3): T = v
def consume(v: T) = v.a
inline def consumeInl(v: T) = v.a
def verify(v: T) = {
assert(consume(v) == 10)
assert(consumeInl(v) == 10)
assert(v.a == 10)
}

def test(): Unit = {
verify(upcast1(new Foo1 { val a = 10 }))
verify(upcast2(new Foo2 { val a = 10 }))
verify(upcast3(new Foo3 { var a = 10 }))

verify(upcast1(new FooBar1))
verify(upcast2(new FooBar2))
verify(upcast3(new FooBar3))

verify(new {val a = 10} : T)
verify(new {var a = 10} : T)
verify(new {def a = 10} : T)

verify(new Bar1 : T)
verify(new Bar2 : T)
verify(new Bar3 : T)
}
}

object TestStructuralVar {
type T = {val a: Int; def a_=(x: Int): Unit}
def upcast3(v: Foo3): T = v
def consume(v: T) = v.a
inline def consumeInl(v: T) = v.a
def verify(v: T) = {
assert(consume(v) == 10)
assert(consumeInl(v) == 10)
assert(v.a == 10)
// Pending, per https://github.com/lampepfl/dotty/issues/4528.
// v.a = 11
// assert(consume(v) == 11)
// assert(consumeInl(v) == 11)
// assert(v.a == 11)
}

def test(): Unit = {
verify(upcast3(new Foo3 { var a = 10 }))
verify(upcast3(new FooBar3))
verify(new {var a = 10} : T)
verify(new Bar3 : T)
}
}

def main(args: Array[String]): Unit = {
TestStructuralVal.test()
TestStructuralDef.test()
TestStructuralVar.test()
}
}