RFR: JDK-8216558: Lookup.unreflectSetter(Field) fails to throw IllegalAccessException for final fields

[Mandy Chung]

Hi Adam,

On 1/30/19 7:52 AM, Adam Farley8 wrote:
> Hi Mandy,
> 
> CSR has been raised: https://bugs.openjdk.java.net/browse/JDK-8218061

Thanks for doing it.  A couple comments:

I think the compatibility risk should be low rather than minimal.
Even the code shouldn't be doing that, unreflectSetter does
allow to set a static final field in the current implementation.
It's low because Field::set throws exception if it's a static
final field and so it's expected that very few existing libraries
would use unreflectSetter.

FWIW.  There are few existing libraries hacking the reflection
to change their static final fields but very few.

Since the spec diff is small, I suggest to cut-n-paste the diff
on the javadoc to the specification section and easier to see
the spec change.  webrev is not generally needed for CSR but
welcome to include that if it helps the review.

I made some minor edit to the CSR to make the problem and solution
clearer.

> The webrev I used includes John's comments, your additions, the 
> one-line-IF,
> and I also took the liberty of moving the unreflectField method underneath
> the unreflectSetter method, because it seemed strange that it was located
> between unreflectSetter and unreflectGetter.
> 
> Online webrev has been updated to include these changes:
> http://cr.openjdk.java.net/~afarley/8216558/webrev

Looks good but the patch still needs a regression test. Please
include the regression test for code review.

Formatting nit: a space is missing between 'if' and '(' and
plase use the 4-space identation of the throw.

1901             if(isSetter && field.isStatic() && field.isFinal())
1902                     throw field.makeAccessException("static final 
field has no write access", this);

Mandy