Skip to content

SI-6181 method mirrors now support by-name args#1052

Closed
xeno-by wants to merge 3 commits intoscala:2.10.xfrom
scalamacros:ticket/6181
Closed

SI-6181 method mirrors now support by-name args#1052
xeno-by wants to merge 3 commits intoscala:2.10.xfrom
scalamacros:ticket/6181

Conversation

@xeno-by
Copy link
Contributor

@xeno-by xeno-by commented Aug 4, 2012

Arguments provided in by-name positions are now automatically wrapped
in Function0 instances by method mirrors.

xeno-by added 2 commits August 4, 2012 16:28
In Scala there are some methods that only exist in symbol tables,
but don't have corresponding method entries in Java class files.

To the best of my knowledge, these methods can be subdivided into four groups:
1) stuff weaved onto Any, AnyVal and AnyRef,
2) magic methods that Scala exposes to fix Java arrays,
3) compile-time methods (such as classOf and all kinds of macros),
4) miscellaneous stuff (currently only String_+).

To support these magic symbols, I've modified the `checkMemberOf` validator
to special case Any/AnyVal/AnyRef methods and adjusted MethodMirror and
ConstructorMirror classes to use special invokers for those instead of
relying on Java reflection.
Arguments provided in by-name positions are now automatically wrapped
in Function0 instances by method mirrors.
@xeno-by
Copy link
Contributor Author

xeno-by commented Aug 4, 2012

The pull request depends on #1048, because both of them touch the method mirror, so separating them would lead to merge conflicts.

@xeno-by
Copy link
Contributor Author

xeno-by commented Aug 4, 2012

review by @adriaanm or @paulp

@scala-jenkins
Copy link

Started jenkins job pr-rangepos at https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/60/

@scala-jenkins
Copy link

Started jenkins job pr-scala-testsuite-linux-opt at https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/764/

@scala-jenkins
Copy link

jenkins job pr-rangepos: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/60/

Copy link
Member

Choose a reason for hiding this comment

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

varargs can also match when fewer arguments are provided.

scala> def foo(a: Any*) = a; foo()
foo: (a: Any*)Seq[Any]
res13: Seq[Any] = List()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, hence the minus one

Copy link
Contributor

Choose a reason for hiding this comment

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

Assuming paramss is flattening multiple parameter lists, here's one which needs a -3.

scala> def f(xs: Any*)(ys: Any*)(zs: Any*) = 5
f: (xs: Any*)(ys: Any*)(zs: Any*)Int

scala> f()()()
res0: Int = 5

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Multiple vararg parameter lists are not supported anyway: https://issues.scala-lang.org/browse/SI-6182. I figured we can do minus three when fixing that issue, and for now we are fine.

@scala-jenkins
Copy link

jenkins job pr-scala-testsuite-linux-opt: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/764/

Copy link
Contributor

Choose a reason for hiding this comment

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

unreachable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@xeno-by
Copy link
Contributor Author

xeno-by commented Aug 5, 2012

PLS REBUILD ALL

@scala-jenkins
Copy link

Started jenkins job pr-rangepos at https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/72/

@scala-jenkins
Copy link

Started jenkins job pr-scala-testsuite-linux-opt at https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/775/

@scala-jenkins
Copy link

jenkins job pr-rangepos: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/72/

@scala-jenkins
Copy link

jenkins job pr-scala-testsuite-linux-opt: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/775/

@odersky
Copy link
Contributor

odersky commented Aug 6, 2012

I think this could use some staging. Performance is critical for method mirror application. E.g. like this:

def transform(ps: List[Params]): List[Any] => List[Any] =
if (ps exists isByNameParam)
???
else
Predef.identity

lazy val trans = transform(ps)

invoke(meth, trans(args))

@xeno-by
Copy link
Contributor Author

xeno-by commented Aug 6, 2012

Will be resubmitted jointly with #1048, #1052, #1054 and #1057.

@xeno-by xeno-by closed this Aug 6, 2012
@xeno-by
Copy link
Contributor Author

xeno-by commented Aug 6, 2012

Superceded by #1067

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants