On 05/11/2018 06:09 PM, mandy chung wrote:
On 4/30/18 10:21 AM, Alan Bateman wrote:
The updated webrev looks good. A minor comment is that I assume you
can remove the cast from Executable::declaredAnnotations if you leave
Executable::getRoot in place.
It could but leave it as is. I found that this change breaks the hack
that uses reflection to change a static final field by changing the
private modifiers field in the Field object. That is a terrible hack
but I think it's better to separate this incompatibility from this
issue. I modified the fix to change the modifiers of the root and
child field object be the same.
http://cr.openjdk.java.net/~mchung/jdk11/webrevs/8202113/webrev.02/
Mandy
Hi Mandy,
Sorry for noticing this too late, but...
If it was not necessary to keep legacy hacky behavior (to honor the
patched "modifiers" field), wouldn't it be cleaner to leave the
ReflectionFactory as is, but modify the following private methods instead:
- Field#acquireFieldAccessor
- Method#acquireMethodAccessor
- Constructor#acquireConstructorAccessor
They already deal with 'root' object and could pass it to
ReflectionFactory. The logic of ReflectionFactory need not deal with the
notion of 'root' object then and no LangReflectAccess additions are
necessary. You would keep the notion of root objects encapsulated. With
tour patch it has leaked into ReflectionFactory too.
Is it really that important to allow users to modify static final fields
that way? As such fields are normally constant folded by JIT, I doubt
that anybody is doing it nowadays. Doing it is bound to unpredictable
program behavior, as JVM is free to never reload such field's value.
Regards, Peter
