Skip to content

Conversation

@xeno-by
Copy link
Contributor

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

Supercedes #1048, #1052, #1054 and #1057.

@scala-jenkins
Copy link

@scala-jenkins
Copy link

@retronym
Copy link
Member

retronym commented Aug 6, 2012

@xeno-by if you hash-reference other pull requests, like #1048, #1052, #1054, #1057, the obedient robotic octocats will insert hrefs back and forth.

@xeno-by
Copy link
Contributor Author

xeno-by commented Aug 6, 2012

neat, thanks!

@xeno-by
Copy link
Contributor Author

xeno-by commented Aug 6, 2012

review by @odersky, @adriaanm and @paulp

@scala-jenkins
Copy link

xeno-by added 5 commits August 6, 2012 23:09
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.
@scala-jenkins
Copy link

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.
@xeno-by
Copy link
Contributor Author

xeno-by commented Aug 6, 2012

PLS REBUILD ALL

@scala-jenkins
Copy link

@scala-jenkins
Copy link

@scala-jenkins
Copy link

@scala-jenkins
Copy link

Copy link
Contributor

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

@odersky
Copy link
Contributor

odersky commented Aug 7, 2012

If the two inline comments are taken care of it will LGTM

@xeno-by
Copy link
Contributor Author

xeno-by commented Aug 7, 2012

PLS REBUILD ALL

@scala-jenkins
Copy link

@scala-jenkins
Copy link

@scala-jenkins
Copy link

adriaanm pushed a commit that referenced this pull request Aug 7, 2012
…ull-request

Ultimate reflection pull request #2
@adriaanm adriaanm merged commit bd71c84 into scala:2.10.x Aug 7, 2012
@scala-jenkins
Copy link

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.

5 participants