RFR: JDK-8216558: Lookup.unreflectSetter(Field) fails to throw IllegalAccessException for final fields
adam.farley at uk.ibm.com
Thu Mar 7 17:44:13 UTC 2019
Since you have created a new work item to cover the test work, do let
me know if you want the test code removed from the webrev for the
Also, by "nobody is meant to be changing final fields anyway", I was
explaining my decision to setAccessible(true) on the final fields by
default, rather than setting it via a control bit as you suggested.
Seemed to save code while exposing us to minimal risk, because the
field is meant to be immutable (final).
Lastly, I'm sorry to see you weren't happy with the quality of my test
code. Your changes seem much larger in scale, and quite different to
my changes. Could you explain the benefits of your approach, vs simply
adding non-static final fields to my code?
No judgement, just looking to learn here. :)
Mandy Chung <mandy.chung at oracle.com> wrote on 07/03/2019 15:38:48:
> From: Mandy Chung <mandy.chung at oracle.com>
> To: Adam Farley8 <adam.farley at uk.ibm.com>
> Cc: core-libs-dev <core-libs-dev at openjdk.java.net>
> Date: 07/03/2019 15:40
> Subject: Re: RFR: JDK-8216558: Lookup.unreflectSetter(Field) fails
> to throw IllegalAccessException for final fields
> Hi Adam,
> On 3/6/19 8:28 AM, Adam Farley8 wrote:
> > Hi Mandy,
> > The webrev has been updated with the new test:
> > https://urldefense.proofpoint.com/v2/url?
> Looks okay although I think the test adds isFinal check for the new test
> case is meant to be say static final field.
> > Note that I included a handful of small improvements, and that the
> > fields are all setAccessible by default, because (a) it seemed cleaner
> > than adding a new control bit, and (b) nobody is meant to be changing
> > final fields anyway.
> I'm not sure what you tried to say about "nobody is meant to be changing
> final fields". There should be tests covering all cases.
> In any case, since you chose to modify MethodHandlesGeneralTest, it's
> okay to add tests for the static final fields which is what this fix
> is about. I created a JBS issue to add test cases for the instance
> final fields:
> FYI. I have a patch for it and will send out for review.
Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
More information about the core-libs-dev