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

[Mandy Chung]

Hi Adam,

You can add a new test for this specific test case.
MethodHandlesGeneralTest is a general test that you either update
it to fit in existing testing framework or add a new test which
will be simpler for you.  Please include the test in the webrev
for easier review.

Mandy

On 3/1/19 2:31 AM, Adam Farley8 wrote:
> 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.chung at oracle.com> wrote on 21/02/2019 17:37:54:
> 
>> 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: 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