Add scala.Dynamic support. #1291

nicolasstucki · 2016-06-01T16:05:43Z

No description provided.

nicolasstucki · 2016-06-01T16:06:51Z

Still need to add some tests and write some comments.

sjrd · 2016-06-01T16:30:06Z

src/dotty/tools/dotc/typer/Applications.scala

+            case Apply(Select(qual, name), args) if !isDynamicMethod(name) =>
+              if (untpd.isWildcardStarArgList(args)) {
+                ctx.error(name.toString + " does not support passing a vararg parameter", tree.pos)
+                tree.withType(ErrorType)


Is there any reason to "preserve" that limitation?

The current implementation does what the scalac implementation does. Im not sure why the restriction was there in the first place. It could be just because it is harder to handle with applyDynamicNamed.

nicolasstucki · 2016-06-02T07:02:09Z

All dynamic method (like selectDynamic(name: String)) receive as first parameter the name of the method. Currently it would be possible to define it as selectDynamic(name: Int) which will always generate an error at call site. I'm considering adding a new check that forces that first type to be a String at the declaration site and generates an error there (could also be a warning). Is anyone against adding that new restriction for some reason?

sjrd · 2016-06-02T07:18:07Z

Yes, I'm against forcing String. You could have an implicit conversion from String to T that should kick in at that point. Scalac sort of supports that, although it's buggy for updateDynamic.

Speaking of which, it would be good to test that implicit conversions can be applied to the arguments at least. js.Dynamic is full of that requirement.

felixmulder · 2016-06-02T19:44:19Z

This PR should solve #657 when merged

smarter · 2016-06-03T13:19:28Z

tests/run/dynamicDynamicTests.scala

@@ -1,3 +1,4 @@
+import dotty.language.dynamics


Unless this works very differently from dynamic in scalac, I think we should use import scala.language.dynamics for compatibility

nicolasstucki · 2016-06-04T09:15:39Z

tests/neg/dynamicApplyDynamicTest8.scala

+}
+
+object DynamicTest {
+  def test: String = new Foo().bazApply("1" -> 2)  // error // error


Note to self: remove one of the // errors

odersky · 2016-06-08T13:13:14Z

src/dotty/tools/dotc/core/Types.scala

+  abstract class TryDynamicCallType extends UncachedGroundType with ValueType
+
+  object TryDynamicCallType extends TryDynamicCallType
+


What happens if we try to map or fold over such a type? Did we think of that?

The idea is that it behaves like ErrorType on any operation until it is caught and handled. I considered making it a subtype of ErrorType but this seamed a bit dangerous and with this alternative there was only one place where I had to add it to a case to do the same as ErrorType.

Actually, now that I think about it, it should be object TryDynamicCallType extends ErrorType.

odersky · 2016-06-30T12:23:50Z

src/dotty/tools/dotc/reporting/Reporter.scala

@@ -206,6 +230,10 @@ abstract class Reporter extends interfaces.ReporterResult {
   */
  def errorsReported = hasErrors

+  private[this] var reportedFeaturesUseSites = Set[Symbol]()


It would be good to have a motivation why this is needed. In fact I am not yet sure we want to suppress feature warnings per use site.

Currently it does not suppress the warning. It is only there to make sure the explanation of how to avoid it is only printed once per site.

Ah, OK, I had missed that.

nicolasstucki · 2016-07-01T12:26:56Z

src/dotty/tools/dotc/typer/ProtoTypes.scala

+      extends UncachedGroundType with AssigningProto {
+    override def resultType(implicit ctx: Context) = rhsType
+
+    def isMatchedBy(tp: Type)(implicit ctx: Context) = ???


@odersky, i'm not sure what to return AssignProto. isMatchedBy.

odersky · 2016-07-06T14:46:30Z

src/dotty/tools/dotc/typer/TypeAssigner.scala

@@ -196,11 +196,16 @@ trait TypeAssigner {
  def selectionType(site: Type, name: Name, pos: Position)(implicit ctx: Context): Type = {
    val mbr = site.member(name)
    if (reallyExists(mbr)) site.select(name, mbr)
-    else {
+    else if (site <:< defn.DynamicType && !Dynamic.isDynamicMethod(name)) {


Cheaper test is: site.derivesFrom(defn.DynamicClass)

nicolasstucki · 2016-07-07T12:42:29Z

Updated

odersky · 2016-07-07T12:51:04Z

LGTM, thanks!

sjrd reviewed Jun 1, 2016
View reviewed changes

felixmulder added the In progress label Jun 2, 2016

nicolasstucki force-pushed the implement-scala-dynamic branch from e4f7c47 to dfe79b6 Compare June 3, 2016 07:25

felixmulder assigned nicolasstucki Jun 3, 2016

nicolasstucki force-pushed the implement-scala-dynamic branch from dfe79b6 to 0920fed Compare June 3, 2016 13:15

smarter reviewed Jun 3, 2016
View reviewed changes

nicolasstucki force-pushed the implement-scala-dynamic branch 2 times, most recently from b4b9b4d to 3d458cf Compare June 3, 2016 15:23

nicolasstucki changed the title ~~[WIP] Add scala.Dynamic support.~~ Add scala.Dynamic support. Jun 3, 2016

nicolasstucki force-pushed the implement-scala-dynamic branch from 3d458cf to c946f73 Compare June 3, 2016 16:06

nicolasstucki reviewed Jun 4, 2016
View reviewed changes

nicolasstucki force-pushed the implement-scala-dynamic branch from c946f73 to 7c39491 Compare June 6, 2016 07:34

nicolasstucki added Needs review and removed In progress labels Jun 6, 2016

nicolasstucki force-pushed the implement-scala-dynamic branch 9 times, most recently from ec7eeca to bf761be Compare June 6, 2016 14:52

nicolasstucki removed their assignment Jun 8, 2016

odersky reviewed Jun 8, 2016
View reviewed changes

nicolasstucki force-pushed the implement-scala-dynamic branch 2 times, most recently from 69c996e to 5efd398 Compare June 9, 2016 12:33

odersky reviewed Jun 30, 2016
View reviewed changes

nicolasstucki force-pushed the implement-scala-dynamic branch from 5efd398 to aebb5dc Compare July 1, 2016 12:18

nicolasstucki reviewed Jul 1, 2016
View reviewed changes

nicolasstucki force-pushed the implement-scala-dynamic branch from aebb5dc to cab2598 Compare July 5, 2016 11:40

odersky reviewed Jul 6, 2016
View reviewed changes

nicolasstucki added 3 commits July 7, 2016 11:10

Fix scala#657: Add scala.Dynamic support.

75da035

Add scala.Dynamic language feature check.

5a7cacb

Fix TypeOps.featureEnabled ignoring import disabling.

7e00c72

nicolasstucki force-pushed the implement-scala-dynamic branch from cab2598 to 7e00c72 Compare July 7, 2016 09:17

dottybot added the reviewed label Jul 7, 2016

DarkDimius removed the Needs review label Jul 13, 2016

DarkDimius merged commit f37e45a into scala:master Jul 15, 2016

DarkDimius removed the reviewed label Jul 15, 2016

		abstract class TryDynamicCallType extends UncachedGroundType with ValueType

		object TryDynamicCallType extends TryDynamicCallType

Add scala.Dynamic support. #1291

Add scala.Dynamic support. #1291

Uh oh!

Conversation

nicolasstucki commented Jun 1, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nicolasstucki commented Jun 1, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nicolasstucki commented Jun 2, 2016

Uh oh!

sjrd commented Jun 2, 2016

Uh oh!

felixmulder commented Jun 2, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nicolasstucki commented Jul 7, 2016

Uh oh!

odersky commented Jul 7, 2016

Uh oh!

Uh oh!

nicolasstucki commented Jun 1, 2016 •

edited

Loading

felixmulder commented Jun 2, 2016 •

edited

Loading