Implement structural type member access #1881

odersky · 2017-01-05T11:03:23Z

New scheme for implementing structural type member access. The main motivation is to use one scheme that works for Java reflection as well as for user-defined access methods, such as accessing database fields where rows are represented as hashmaps.

Details will be explained in a separate issue.

New scheme for implementing structural type member access.

liufengyun · 2017-01-05T20:46:17Z

tests/pos/zoo2.scala

+val milka = newCow
+val leo = newLion
+leo.eats(milka)
+}


The test error suggests that import scala.reflect.Projector.reflectiveProjector is missing in this test.

Use base types instead of implicits. This is more robust in the presence of type abstraction.

odersky · 2017-01-07T12:07:55Z

The new scheme does not use implicit resolution, which makes it simpler and more robust. The old scheme suffered from the following drawback:

import ... .reflectiveProjector
val x: Any = new Record(...)
 x.name   // uses reflective access and fails, since records don't have a field called "name".

odersky · 2017-01-07T12:08:21Z

I think this is ready for review now. Who wants to review?

liufengyun · 2017-01-07T12:43:09Z

I'm happy to review. Maybe @nicolasstucki can also have a look, as it's related to dynamic?

smarter · 2017-01-07T14:05:52Z

compiler/src/dotty/tools/dotc/ast/TreeInfo.scala

+        case tp: TypeProxy =>
+          hasRefinement(tp.underlying)
+        case tp: OrType =>
+          hasRefinement(tp.tp1) || hasRefinement(tp.tp2)


Aren't we mixing up the body of OrType and AndType here? To select foo, foo needs to be present in both sides of an OrType, but only one side of an AndType. In any case, we need some testcases for structural type selection on union/intersection types

It needs to be present on both sides, but one side might be a regular class member. As long as one side comes from a refinement, the access is reflection based. For an AndType, it's the other way round. A single class member on one side is sufficient for regular access.

Ah, I see! The AndType case makes sense, the OrType case seems suspicious but I don't think it matters since we no longer allow calling a method on a union type that matches more than one symbol, e.g. the following doesn't compile:

class Closeable { def close(): Unit = {} } object Test { import scala.reflect.Selectable.reflectiveSelectable def f(closeable: (Selectable { def close(): Unit }) | Closeable) = closeable.close() // error: value `close` is not a member of (Selectable{close: ()Unit} | Closeable)(closeable) def main(args: Array[String]): Unit = { f(new Closeable) } }

smarter

Would be interesting to see if this scheme can be extended to support repeated parameters.

smarter · 2017-01-07T14:07:22Z

compiler/src/dotty/tools/dotc/core/Definitions.scala

@@ -26,6 +26,9 @@ object Definitions {
   *  else without affecting the set of programs that can be compiled.
   */
  val MaxImplementedFunctionArity = 22
+
+  /** The maximal arity of a function thta can be accessed as member of a structrual type */


typo: thta -> that

smarter · 2017-01-07T14:12:28Z

compiler/src/dotty/tools/dotc/typer/Dynamic.scala

+        if (tpe.isDependent)
+          fail(i"has a dependent method type")
+        else if (tpe.paramNames.length > Definitions.MaxStructuralMethodArity)
+          fail(i"takes too many parameters")


I would expand this message to say "Structural types only support methods taking up to ${Definitions.MaxStructuralMethodArity} arguments", to keep people from guessing.

smarter · 2017-01-07T14:23:24Z

library/src/scala/reflect/Selectable.scala

@@ -0,0 +1,73 @@
+package scala.reflect
+
+class Selectable(val receiver: Any) extends AnyVal with scala.Selectable {


I would call this ReflectiveSelectable or something like that, it's easy to miss the prefix.

liufengyun

In addition to @smarter good findings, I have a concern about platform compatibility -- how can user use structural types in a platform-independent way(native, scala.js)?

If I have a library which defines a method below:

def f(x: { def get: Int}) = x.get

If structural types were used as it's implemented now, it will force one particular implementation, thus makes the code above platform(implementation)-dependent.

I'm thinking if it's possible to transform the method above to the following if the type is a structural type:

def f(x: Selectable @{ def get: Int}) = x.get

The explicit structural type requirement becomes annotation for type checking purpose, the type will be changed to Selectable.

This way, the code above will be more portable and implementation-independent.

liufengyun · 2017-01-07T14:42:11Z

compiler/src/dotty/tools/dotc/typer/Dynamic.scala

+   *  If `U` is a value type, map `x.a` to the equivalent of:
+   *
+   *     (x: Selectable).selectDynamic(x, "a").asInstanceOf[U]
+   *


The parameter to selectDynamic is incorrect, should be selectDynamic("a").

liufengyun · 2017-01-07T14:43:24Z

compiler/src/dotty/tools/dotc/typer/Dynamic.scala

+   *
+   *  If `U` is a method type (T1,...,Tn)R, map `x.a` to the equivalent of:
+   *
+   *     (x: Selectable).selectDynamicMethod(x, "a")(CT1, ..., CTn).asInstanceOf[(T1,...,Tn) => R]


The parameter to selectDynamicMethod should be selectDynamicMethod("a", CT1, ..., CTn)

liufengyun · 2017-01-07T14:45:50Z

library/src/scala/Selectable.scala

+package scala
+import scala.reflect.ClassTag
+
+trait Selectable extends Any {


Better to have following annotation:

@implicitNotFound("no Selectable instance found to implement reflective access to structural type ${T}. Forget to import scala.reflect.Selectable.reflectiveSelectable?")

smarter · 2017-01-07T16:04:39Z

To expand on what @liufengyun suggested, currently { def get: Int}, is syntactic sugar for AnyRef { def get: Int }, which means that the type is unusable without an implicit conversion like reflectiveSelectable, but I think it would make more sense to have { def get: Int } be syntactic sugar for Selectable { def get: Int } (which gets erased to Selectable), this encourages people to not use the reflective stuff by default, but the issue is that when we do want to use reflection we would need an implicit conversion like this:

AnyRef { $REFINEMENTS }
         -->
Selectable { $REFINEMENTS }

I don't think we can do that in the library by itself, but maybe with some compiler magic?

liufengyun · 2017-01-07T17:01:31Z

It seems that we don't need reflection to implement structural types.

If we transform the method below:

def f(x: { def get: Int}) = x.get

to

def f(x: Selectable @{ def get: Int}) = x.selectDynamic("get").asInstanceOf[Int]

At the calling site, we can transform the call:

f(Some(5))

to the following:

val x = Some(5)
val d = new Selectable {
    def selectDynamic(name: String): Any = 
        if (name == "get") x.get else error 
    }
}
f(d)

Following the approach above, it seems we don't need reflection at all.

Of course, for backward-compatibility this is not a good idea.

nicolasstucki · 2017-01-09T14:41:50Z

library/src/scala/reflect/Selectable.scala

+            x3.asInstanceOf[Object],
+            x4.asInstanceOf[Object],
+            x5.asInstanceOf[Object],
+            x6.asInstanceOf[Object])


Why do we require functions with fixed sizes? Could we not return an object with an apply(args: Object*) and then pass the args directly to the invoke?

The current solution has the advantage that the result of a Selectable is a drop in replacement of the original call. If we bunched arguments in an array, this would mean we also have the change the calling context. So it's more complicated. Selectable has lots of restrictions anyway, so one more does not really matter.

Ok.

Other related question: is the limit of 7 parameters an arbitrary limit or is there some factor that influenced this limit?

The PR also fixes scala#1866, as shown by this test case.

odersky · 2017-01-10T13:33:16Z

It seems that we don't need reflection to implement structural types.

Have a look at Gilles Dubochet's thesis and at the "WhiteOak" paper he discusses. The WhiteOak paper uses essentially what you propose to avoid reflection. But the details are hairy. So hairy in fact, that the publicly available WhiteOak implementation does not follow the scheme described in their paper and uses reflection instead.

liufengyun · 2017-01-10T13:47:52Z

Thanks for the pointer @odersky , I was thinking I must have missed something there :)

liufengyun · 2017-01-10T16:00:14Z

Three code examples from the paper that fail the reflection-based approach:

Test1: compiles, but runtime error

import scala.reflect.Selectable.reflectiveSelectable

object Test1 {
  def g(x: Any{ def + (i: Int): Int }) = x + 2
  def main(args: Array[String]): Unit = g(3)
}

Test2: doesn't compile

import scala.reflect.Selectable.reflectiveSelectable

object Test2 {
  def g[T](x: { def f(a: T): Boolean }, t: T) = x.f(t)

  g[Int](new { def f(a: Int) = true }, 4)
  g[Any](new { def f(a: Any) = true }, 5)
}

Test3: compiler crash

import scala.reflect.Selectable.reflectiveSelectable

object Test3 {
  def g(x: { type T ; def t: T ; def f(a: T): Boolean }) = x.f(x.t)
  g(new { type T = Int; def t = 4; def f(a:T) = true })
  g(new { type T = Any; def t = 4; def f(a:T) = true })
}

Test4: compiler crash

import scala.reflect.Selectable.reflectiveSelectable

object Test {
  def g(x: { def f[T](a: T): Int }) = x.f[Int](4)
}

I don't think these cases are important, no one will ever write such code. Just document here for information.

Edited: added Test4.

odersky · 2017-01-11T03:30:42Z

@liufengyun Can you make an issue from Test 3? we should track that one.

odersky · 2017-01-11T03:32:36Z

@liufengyun

I have a concern about platform compatibility -- how can user use structural types in a platform-independent way(native, scala.js)?

I believe they would each have different implicit implementations of scala.reflect.Selectable. That should do it, right?

odersky · 2017-01-11T03:35:52Z

@liufengyun @smarter Regarding the meaning of new {...} I think there's a larger question: how are values of structural types generated? Right now we do whatever and then slap a cast to a structural type on it. There should be more systematic ways to do this, probably related to bijections from HMaps/HList. /cc @OlivierBlanvillain.

For the moment I would leave structural creation rules unchanged, waiting for learning more about the issues.

liufengyun · 2017-01-11T08:59:52Z

@odersky

I have a concern about platform compatibility -- how can user use structural types in a platform-independent way(native, scala.js)?

I believe they would each have different implicit implementations of scala.reflect.Selectable. That should do it, right?

Yes, if a library is specially designed for scala.js or native, they can import the platform-specific implementation of Selectable. However, if a library is intended for all platforms, it seems difficult to do that without changing the import scala.xxx.Selectable in library source code for each platform.

odersky · 2017-01-28T03:19:19Z

@liufengyun

However, if a library is intended for all platforms, it seems difficult to do that without changing the import scala.xxx.Selectable in library source code for each platform.

That's precisely what I suggest: Change the implementation of scala.reflect.Selectable for each platform. That's the only possible route. Note that it's the standard library that is changed, not user code.

A previous type comparison was wrong because it did not map refined-this types. I believe it was also redundant, so the easiest fix is to drop it.

We can't handle them with the proposed scheme. I made a note in scala#1886.

odersky · 2017-01-29T02:46:33Z

Any more suggestions, or can we merge?

Implement structural type member access

aa6ebe9

New scheme for implementing structural type member access.

odersky added the stat:in progress label Jan 5, 2017

Cleanups

7e3f69a

liufengyun reviewed Jan 5, 2017

View reviewed changes

Change scheme to use Selectable

69feaa8

Use base types instead of implicits. This is more robust in the presence of type abstraction.

odersky mentioned this pull request Jan 7, 2017

Rethink Structural Types #1886

Closed

odersky added stat:needs review and removed stat:in progress labels Jan 7, 2017

smarter reviewed Jan 7, 2017

View reviewed changes

liufengyun reviewed Jan 7, 2017

View reviewed changes

nicolasstucki reviewed Jan 9, 2017

View reviewed changes

Add test case for scala#1866

bb81d5d

The PR also fixes scala#1866, as shown by this test case.

Address reviewer comments

8464c16

liufengyun mentioned this pull request Jan 11, 2017

Compiler crash while compiling structural types with type member #1897

Closed

Generalize test whether access is to a refinement

2bbf9ca

A previous type comparison was wrong because it did not map refined-this types. I believe it was also redundant, so the easiest fix is to drop it.

Disallow polymorphic refinements in stuctural types.

678e8e4

We can't handle them with the proposed scheme. I made a note in scala#1886.

odersky merged commit af7fdb3 into scala:master Feb 1, 2017

odersky removed the stat:needs review label Feb 1, 2017

smarter mentioned this pull request Apr 11, 2017

Decide upon approach to structural types #1517

Closed

allanrenucci deleted the add-structural-select branch December 14, 2017 16:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement structural type member access #1881

Implement structural type member access #1881

odersky commented Jan 5, 2017

liufengyun Jan 5, 2017

odersky commented Jan 7, 2017

odersky commented Jan 7, 2017

liufengyun commented Jan 7, 2017

smarter Jan 7, 2017 •

edited

Loading

odersky Jan 7, 2017

smarter Jan 8, 2017 •

edited

Loading

smarter left a comment

smarter Jan 7, 2017

smarter Jan 7, 2017

smarter Jan 7, 2017

liufengyun left a comment

liufengyun Jan 7, 2017

liufengyun Jan 7, 2017

liufengyun Jan 7, 2017

smarter commented Jan 7, 2017 •

edited

Loading

liufengyun commented Jan 7, 2017 •

edited

Loading

nicolasstucki Jan 9, 2017

odersky Jan 10, 2017

nicolasstucki Jan 10, 2017

odersky commented Jan 10, 2017 •

edited

Loading

liufengyun commented Jan 10, 2017

liufengyun commented Jan 10, 2017 •

edited

Loading

odersky commented Jan 11, 2017

odersky commented Jan 11, 2017 •

edited

Loading

odersky commented Jan 11, 2017

liufengyun commented Jan 11, 2017 •

edited

Loading

odersky commented Jan 28, 2017 •

edited

Loading

odersky commented Jan 29, 2017

		@@ -0,0 +1,73 @@
		package scala.reflect

		class Selectable(val receiver: Any) extends AnyVal with scala.Selectable {

Implement structural type member access #1881

Implement structural type member access #1881

Conversation

odersky commented Jan 5, 2017

Choose a reason for hiding this comment

odersky commented Jan 7, 2017

odersky commented Jan 7, 2017

liufengyun commented Jan 7, 2017

smarter Jan 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smarter Jan 8, 2017 • edited Loading

Choose a reason for hiding this comment

smarter left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liufengyun left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smarter commented Jan 7, 2017 • edited Loading

liufengyun commented Jan 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

odersky commented Jan 10, 2017 • edited Loading

liufengyun commented Jan 10, 2017

liufengyun commented Jan 10, 2017 • edited Loading

odersky commented Jan 11, 2017

odersky commented Jan 11, 2017 • edited Loading

odersky commented Jan 11, 2017

liufengyun commented Jan 11, 2017 • edited Loading

odersky commented Jan 28, 2017 • edited Loading

odersky commented Jan 29, 2017

smarter Jan 7, 2017 •

edited

Loading

smarter Jan 8, 2017 •

edited

Loading

smarter commented Jan 7, 2017 •

edited

Loading

liufengyun commented Jan 7, 2017 •

edited

Loading

odersky commented Jan 10, 2017 •

edited

Loading

liufengyun commented Jan 10, 2017 •

edited

Loading

odersky commented Jan 11, 2017 •

edited

Loading

liufengyun commented Jan 11, 2017 •

edited

Loading

odersky commented Jan 28, 2017 •

edited

Loading