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

[Adam Farley8]

Hi All,

Some comments below.

> >>>> A CSR request will need to be filed.
> >>>>
> >>>
> >>> Of course, as this is a spec change.
> >>
> >> I'm unclear what spec is actually to be changed here and in what way? 


As am I.

> > 
> > I expect Adam will send a revised webrev to include the proposed spec 
> > change.

I've updated the webrev to include Mandy's MemberName.isStatic code, but
I've kept the modifier analysis near the surface to avoid domino effects
on other code.

This way we can modify the setter method's output with minimal risk.

As for the CSR process, I'm unfamiliar with it. I've modified the 
comment for the unreflectSetter method in the webrev. Do let me know if 
more needs to be done.

> >> If these methods produce MH that obey bytecode semantics then the 
fact 
> >> the Field had setAccessible(true) called on it is not relevant - the 
> >> JVMS knows nothing about setAccessible. The only issue I see is the 
> >> implementation not throwing IAE when the Field represents a final 
> >> field, because it examines the "accessibility" state. (And that 
should 
> >> apply whether the field is static or not.)
> > 
> > This is the current implementation matching the specification:
> >      If the field's accessible flag is not set, access checking is 
> > performed immediately on behalf of the lookup class.
> > 
> > I think it's intended to match Field::setAccessible behavior but 
> > Field::setAccessible has no effect in suppressing the language access 
> > check in static final field.  Hence I agree with Adam's fix changes 
that 
> > the access check should be performed for static final field even its 
> > accessible flag is set.
> 
> I think there is a problem knowing when "access check" means just access 

> check and when it means "access check plus the special hack for setting 
> final fields". I'm not reading any final field semantics into the 
> existing text, because bytecode does not know about setAccessible and 
> final fields. I can see an argument to be made for the case where 
> setAccessible(true) has been called to disable language level access 
> checks, and you want the MH to follow the same access rules. But that 
> does not to me imply that the ability to write to a final field should 
> be part of that. If that is intended then it needs to be very, very 
> clearly spelt out.
> 
> And as per my other email there is already a problem with the bytecode 
> equivalence of MH semantics and final fields, because you can set final 
> fields in <init> or <clinit> (though only once). I don't think MH 
> captures that - nor can it. Which means we can't defer to bytecode 
> semantics here but rather have to very clearly define how MH interact 
> with final fields.
> 
> David
> 
> > Mandy
> 

Adam

Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598. 
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU