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

Allow selectDynamic and applyDynamic to be extension methods #17106

Merged
merged 3 commits into from
Mar 17, 2023
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
10 changes: 5 additions & 5 deletions compiler/src/dotty/tools/dotc/typer/Dynamic.scala
Original file line number Diff line number Diff line change
Expand Up @@ -181,12 +181,12 @@ trait Dynamic {
val vargss = termArgss(tree)

def structuralCall(selectorName: TermName, classOfs: => List[Tree]) = {
val selectable = adapt(qual, defn.SelectableClass.typeRef)
val selectable = adapt(qual, defn.SelectableClass.typeRef | defn.DynamicClass.typeRef)

// ($qual: Selectable).$selectorName("$name")
val base =
untpd.Apply(
untpd.TypedSplice(selectable.select(selectorName)).withSpan(fun.span),
untpd.Select(untpd.TypedSplice(selectable), selectorName).withSpan(fun.span),
Copy link
Member

Choose a reason for hiding this comment

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

That will indeed allow extension methods, but won't it also allow all sorts of other things? Like recursive structural lookups and insertion of implicit conversions? That seems too much (especially the recursive structural lookup).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It will allow implicit conversions, and that's intentional. It will allow recursive structural lookup if selectDynamic is defined in the structural type. But I think that's a weird corner case, and I also see nothing wrong to support it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

EDIT: That should have been applyDynamic.

(Literal(Constant(name.encode.toString)) :: Nil).map(untpd.TypedSplice(_)))

val scall =
Expand Down Expand Up @@ -219,19 +219,19 @@ trait Dynamic {
extension (tree: Tree)
/** The implementations of `selectDynamic` and `applyDynamic` in `scala.reflect.SelectDynamic` have no information about the expected return type of a value/method which was declared in the refinement,
* only the JVM type after erasure can be obtained through reflection, e.g.
*
*
* class Foo(val i: Int) extends AnyVal
* class Reflective extends reflect.Selectable
* val reflective = new Reflective {
* def foo = Foo(1) // Foo at compile time, java.lang.Integer in reflection
* }
*
*
* Because of that reflective access cannot be implemented properly in `scala.reflect.SelectDynamic` itself
* because it's not known there if the value should be wrapped in a value class constructor call or not.
* Hence the logic of wrapping is performed here, relying on the fact that the implementations of `selectDynamic` and `applyDynamic` in `scala.reflect.SelectDynamic` are final.
*/
def maybeBoxingCast(tpe: Type) =
val maybeBoxed =
val maybeBoxed =
if ValueClasses.isDerivedValueClass(tpe.classSymbol) && qual.tpe <:< defn.ReflectSelectableTypeRef then
val genericUnderlying = ValueClasses.valueClassUnbox(tpe.classSymbol.asClass)
val underlying = tpe.select(genericUnderlying).widen.resultType
Expand Down
14 changes: 14 additions & 0 deletions tests/pos/i17100.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
trait Sel extends Selectable

extension (s: Sel)
def selectDynamic(name: String) = ???
def applyDynamic(name: String)(x: Int) = ???
def applyDynamic(name: String)() = ???

val sel = (new Sel {}).asInstanceOf[Sel{ def foo: String; def bar(x: Int): Int; def baz(): Int }]
val foo = sel.selectDynamic("foo")
val foo2 = sel.foo
val foo3 = sel.bar(2)
val foo4 = sel.baz()


12 changes: 12 additions & 0 deletions tests/pos/i17100a.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@

import scala.language.dynamics
trait Sel extends Dynamic

extension (s: Sel)
def selectDynamic(name: String) = ???

val sel = new Sel {}
val foo = sel.foo
val sel2 = (new Sel {}).asInstanceOf[Sel{ def foo: String }]
val foo2 = sel2.foo

8 changes: 4 additions & 4 deletions tests/semanticdb/expect/Advanced.expect.scala
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ class Wildcards/*<-advanced::Wildcards#*/ {
object Test/*<-advanced::Test.*/ {
val s/*<-advanced::Test.s.*/ = new Structural/*->advanced::Structural#*/
val s1/*<-advanced::Test.s1.*/ = s/*->advanced::Test.s.*/.s1/*->advanced::Structural#s1().*/
val s1x/*<-advanced::Test.s1x.*/ = s/*->advanced::Test.s.*/.s1/*->advanced::Structural#s1().*/.x
val s1x/*<-advanced::Test.s1x.*/ = s/*->advanced::Test.s.*/.s1/*->advanced::Structural#s1().*/.x/*->scala::reflect::Selectable#selectDynamic().*/
val s2/*<-advanced::Test.s2.*/ = s/*->advanced::Test.s.*/.s2/*->advanced::Structural#s2().*/
val s2x/*<-advanced::Test.s2x.*/ = s/*->advanced::Test.s.*/.s2/*->advanced::Structural#s2().*/.x
val s2x/*<-advanced::Test.s2x.*/ = s/*->advanced::Test.s.*/.s2/*->advanced::Structural#s2().*/.x/*->scala::reflect::Selectable#selectDynamic().*/
val s3/*<-advanced::Test.s3.*/ = s/*->advanced::Test.s.*/.s3/*->advanced::Structural#s3().*/
val s3x/*<-advanced::Test.s3x.*/ = s/*->advanced::Test.s.*/.s3/*->advanced::Structural#s3().*/.m(???/*->scala::Predef.`???`().*/)
val s3x/*<-advanced::Test.s3x.*/ = s/*->advanced::Test.s.*/.s3/*->advanced::Structural#s3().*/.m/*->scala::reflect::Selectable#applyDynamic().*/(???/*->scala::Predef.`???`().*/)

val e/*<-advanced::Test.e.*/ = new Wildcards/*->advanced::Wildcards#*/
val e1/*<-advanced::Test.e1.*/ = e/*->advanced::Test.e.*/.e1/*->advanced::Wildcards#e1().*/
Expand All @@ -45,7 +45,7 @@ object Test/*<-advanced::Test.*/ {

// see: https://github.com/lampepfl/dotty/pull/14608#discussion_r835642563
lazy val foo/*<-advanced::Test.foo.*/: (reflect.Selectable/*->scala::reflect::Selectable#*/ { type A/*<-local16*/ = Int/*->scala::Int#*/ }) &/*->scala::`&`#*/ (reflect.Selectable/*->scala::reflect::Selectable#*/ { type A/*<-local17*/ = Int/*->scala::Int#*/; val a/*<-local18*/: A/*->local17*/ }) = ???/*->scala::Predef.`???`().*/
def bar/*<-advanced::Test.bar().*/: foo/*->advanced::Test.foo.*/.A/*->local17*/ = foo/*->advanced::Test.foo.*/.a
def bar/*<-advanced::Test.bar().*/: foo/*->advanced::Test.foo.*/.A/*->local17*/ = foo/*->advanced::Test.foo.*/.a/*->scala::reflect::Selectable#selectDynamic().*/
}


Expand Down
6 changes: 5 additions & 1 deletion tests/semanticdb/metac.expect
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ Uri => Advanced.scala
Text => empty
Language => Scala
Symbols => 60 entries
Occurrences => 134 entries
Occurrences => 138 entries
Synthetics => 3 entries

Symbols:
Expand Down Expand Up @@ -187,18 +187,21 @@ Occurrences:
[27:6..27:9): s1x <- advanced/Test.s1x.
[27:12..27:13): s -> advanced/Test.s.
[27:14..27:16): s1 -> advanced/Structural#s1().
[27:16..27:18): .x -> scala/reflect/Selectable#selectDynamic().
[28:6..28:8): s2 <- advanced/Test.s2.
[28:11..28:12): s -> advanced/Test.s.
[28:13..28:15): s2 -> advanced/Structural#s2().
[29:6..29:9): s2x <- advanced/Test.s2x.
[29:12..29:13): s -> advanced/Test.s.
[29:14..29:16): s2 -> advanced/Structural#s2().
[29:16..29:18): .x -> scala/reflect/Selectable#selectDynamic().
[30:6..30:8): s3 <- advanced/Test.s3.
[30:11..30:12): s -> advanced/Test.s.
[30:13..30:15): s3 -> advanced/Structural#s3().
[31:6..31:9): s3x <- advanced/Test.s3x.
[31:12..31:13): s -> advanced/Test.s.
[31:14..31:16): s3 -> advanced/Structural#s3().
[31:16..31:18): .m -> scala/reflect/Selectable#applyDynamic().
[31:19..31:22): ??? -> scala/Predef.`???`().
[33:6..33:7): e <- advanced/Test.e.
[33:14..33:23): Wildcards -> advanced/Wildcards#
Expand Down Expand Up @@ -233,6 +236,7 @@ Occurrences:
[47:11..47:14): foo -> advanced/Test.foo.
[47:15..47:16): A -> local17
[47:19..47:22): foo -> advanced/Test.foo.
[47:22..47:24): .a -> scala/reflect/Selectable#selectDynamic().
[52:6..52:13): HKClass <- advanced/HKClass#
[52:14..52:15): F <- advanced/HKClass#[F]
[52:20..52:21): T <- advanced/HKClass#`<init>`().[F][T]
Expand Down