-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Ultimate reflection pull request #2 #1067
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
Ultimate reflection pull request #2 #1067
Conversation
|
Started jenkins job pr-rangepos at https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/89/ |
|
Started jenkins job pr-scala-testsuite-linux-opt at https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/792/ |
|
neat, thanks! |
|
jenkins job pr-rangepos: Failed - https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/89/ |
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 five groups: 1) stuff weaved onto Any, AnyVal and AnyRef (aka Object), 2) magic methods that Scala exposes to fix Java arrays, 3) magic methods declared on Scala primitive value classes, 4) compile-time methods (such as classOf and all kinds of macros), 5) 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. Support for value classes will arrive in the subsequent commit, because it requires some unrelated changes to the mirror API (currently mirrors only support AnyRefs as their targets).
mirrors now carry a class tag of the receiver, so that they can detect value classes being reflected upon and adjust accordingly (e.g. allow Int_+ for ints, but disallow it for Integers). Surprisingly enough derived value classes (SIP-15 guys that inherit from AnyVal) have been working all along, so no modification were required to fix them.
Arguments provided in by-name positions are now automatically wrapped in Function0 instances by method mirrors.
In 911bbc4 I've completely overlooked the fact that reflectConstructor exists and that is also needs sanity checks. Now reflectConstructor checks that the incoming symbol is actually a ctor, and that it is actually a ctor of the class reflected by the current mirror.
Previously `checkMemberOf` was blocking base fields and methods that are overriden in receiver.getClass. Now this is fixed. The fix also uncovered an issue with field mirrors. Currently their `get` and `set` methods don't respect overriding and always return field values from a base class. After discussing this on a reflection meeting, we decided that this behavior is desirable and that for overriding people should use reflectMethod and then apply on getters/setters. See the discussion at: scala#1054.
|
jenkins job pr-scala-testsuite-linux-opt: Failed - https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/792/ |
Since Scala reflection relies on Java reflection to perform member invocations, it inherits some of the quirks of the underlying platform. One of such quirks is returning null when invoking a void-returning method. This is now fixed by introducing a check after calling invoke.
|
PLS REBUILD ALL |
|
Started jenkins job pr-rangepos at https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/92/ |
|
Started jenkins job pr-scala-testsuite-linux-opt at https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/795/ |
|
jenkins job pr-rangepos: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/92/ |
|
jenkins job pr-scala-testsuite-linux-opt: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/795/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can't you write:
owner.info.baseClasses contains wannabe.owner
|
If the two inline comments are taken care of it will LGTM |
|
PLS REBUILD ALL |
|
Started jenkins job pr-rangepos at https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/105/ |
|
Started jenkins job pr-scala-testsuite-linux-opt at https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/809/ |
|
jenkins job pr-rangepos: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/105/ |
…ull-request Ultimate reflection pull request #2
|
jenkins job pr-scala-testsuite-linux-opt: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/809/ |
Supercedes #1048, #1052, #1054 and #1057.