Hi Mandy, Historically, the bigger the change I propose, the more months it takes the OpenJDK community to approve.
I'm not sure I can justify adding an entire class to test a very specific case. Additionally, HasFields seems excessively complex. I don't understand why it feels the need to spend 34 lines of code parsing, processing, and storing 20 minimal variables it already has. Seems like an informative comment, followed by a littany of "cases.add..." would have been a simpler choice. Could you tell me if I'm missing something? Best Regards Adam Farley IBM Runtimes Mandy Chung <mandy.ch...@oracle.com> wrote on 21/02/2019 17:37:54: > From: Mandy Chung <mandy.ch...@oracle.com> > To: Adam Farley8 <adam.far...@uk.ibm.com> > Cc: core-libs-dev <core-libs-dev@openjdk.java.net> > Date: 21/02/2019 17:41 > Subject: Re: RFR: JDK-8216558: Lookup.unreflectSetter(Field) fails > to throw IllegalAccessException for final fields > > Hi Adam, > > On 2/14/19 3:16 AM, Adam Farley8 wrote: > > Hi Mandy, > > > > Apologies for the delay. > > Same here as I returned from vacation yesterday. > > > Could you review this cdiff as a proposal for the jtreg test? > > > > Made sense to modify the existing test set for MethodHandle rather than > > add a new one. > > Yes it'd be better if you extend the MethodHandlesGeneralTest and > MethodHandlesTest to test the write access. > > To add the test cases properly, MethodHandlesTest.HasFields > should be modified to include the read-only fields. It might > be cleaner to add a new HasReadOnlyFields class and maybe a new > TEST_SET_ACCESSIBLE bit that requests to call setAccessible(true) > and testSetter expects unreflectSetter to throw exception on > static final fields (possibly additional bits might be needed). > > Mandy > 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
