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

On 3/26/19 10:24 AM, Adam Farley8 wrote: Hiya Mandy, I've updated the webrev with your proposal, plus a few tweaks for formatting. http://cr.openjdk.java.net/~afarley/8216558.2/webrev Let me know what you think. This looks okay.  I took the liberty to take out the comment line

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

: Adam Farley8 , Joe Darcy > Cc: core-libs-dev > Date: 26/03/2019 03:51 > Subject: Re: RFR: JDK-8216558: Lookup.unreflectSetter(Field) fails > to throw IllegalAccessException for final fields > > > On 3/25/19 11:36 AM, Adam Farley8 wrote: > Addendum: URL for new webr

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

On 3/25/19 11:36 AM, Adam Farley8 wrote: Addendum: URL for new webrev: http://cr.openjdk.java.net/~afarley/8216558.1/webrev/ Thanks for sending a versioned webrev. > What is a USA test? UNREFLECT_SETTER_ACCESSIBLE. I was trying to be brief, and I lost readability. Will re-upload with

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

On 3/25/2019 4:48 AM, Adam Farley8 wrote: Hiya Joe, Responses below. Joe Darcy wrote on 22/03/2019 17:06:38: > From: Joe Darcy > To: Mandy Chung , Adam Farley8 > > Cc: core-libs-dev > Date: 22/03/2019 17:07 > Subject: Re: RFR: JDK-8216558: Lookup.unreflectSetter(Field

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

y wrote on 22/03/2019 17:05:33: > > > > > From: Joe Darcy > > > To: Adam Farley8 > > > Cc: core-libs-dev , Mandy Chung > > > > > > Date: 22/03/2019 17:06 > > > Subject: Re: RFR: JDK-8216558: Lookup.unreflectSetter(Field) fails &

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

arley8 > Cc: core-libs-dev , Mandy Chung > > Date: 25/03/2019 17:35 > Subject: Re: RFR: JDK-8216558: Lookup.unreflectSetter(Field) fails > to throw IllegalAccessException for final fields > > On 3/25/2019 4:50 AM, Adam Farley8 wrote: > Hiya Joe, > > Response below,

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

On 3/25/2019 4:50 AM, Adam Farley8 wrote: Hiya Joe, Response below, Joe Darcy wrote on 22/03/2019 17:05:33: > From: Joe Darcy > To: Adam Farley8 > Cc: core-libs-dev , Mandy Chung > > Date: 22/03/2019 17:06 > Subject: Re: RFR: JDK-8216558: Lookup.unreflectSetter(Field

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

Hiya Joe, Response below, Joe Darcy wrote on 22/03/2019 17:05:33: > From: Joe Darcy > To: Adam Farley8 > Cc: core-libs-dev , Mandy Chung > > Date: 22/03/2019 17:06 > Subject: Re: RFR: JDK-8216558: Lookup.unreflectSetter(Field) fails > to throw IllegalAccessExcep

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

Hiya Joe, Responses below. Joe Darcy wrote on 22/03/2019 17:06:38: > From: Joe Darcy > To: Mandy Chung , Adam Farley8 > > Cc: core-libs-dev > Date: 22/03/2019 17:07 > Subject: Re: RFR: JDK-8216558: Lookup.unreflectSetter(Field) fails > to throw IllegalAccessExcep

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

Hi Mandy, Mandy Chung wrote on 22/03/2019 16:56:12: > From: Mandy Chung > To: Joe Darcy , Adam Farley8 > Cc: core-libs-dev > Date: 22/03/2019 16:58 > Subject: Re: RFR: JDK-8216558: Lookup.unreflectSetter(Field) fails > to throw IllegalAccessException for final fields >

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

On 3/22/2019 9:56 AM, Mandy Chung wrote: Hi Adam, On 3/22/19 8:40 AM, Joe Darcy wrote: Please update distinct versions of a webrev (e.g. distinguished with .1, .2 directory names) rather than overwriting a single one. This make it easier for those coming to the review thread later to see

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

Hi Adam, On 3/22/2019 9:14 AM, Adam Farley8 wrote: Hi Joe, I was aware that webrevs should be versioned, though I didn't see the value for small change sets like this one. You seem to think there is a value. Can you explain it to me? The time of reviewers is valuable and should not be

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

Hi Adam, On 3/22/19 8:40 AM, Joe Darcy wrote: Please update distinct versions of a webrev (e.g. distinguished with .1, .2 directory names) rather than overwriting a single one. This make it easier for those coming to the review thread later to see the evolution of the changes over time.

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

rcy > To: Adam Farley8 , Mandy Chung > > Cc: core-libs-dev > Date: 22/03/2019 15:42 > Subject: Re: RFR: JDK-8216558: Lookup.unreflectSetter(Field) fails > to throw IllegalAccessException for final fields > > A quick comment below... > > On 3/22/2019 4:33 AM,

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

A quick comment below... On 3/22/2019 4:33 AM, Adam Farley8 wrote: Hi Mandy, Answers below. :) Mandy Chung wrote on 22/03/2019 00:35:00: From: Mandy Chung To: Adam Farley8 Cc: core-libs-dev Date: 22/03/2019 00:35 Subject: Re: RFR: JDK-8216558: Lookup.unreflectSetter(Field) fails

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

Hi Mandy, Answers below. :) Mandy Chung wrote on 22/03/2019 00:35:00: > From: Mandy Chung > To: Adam Farley8 > Cc: core-libs-dev > Date: 22/03/2019 00:35 > Subject: Re: RFR: JDK-8216558: Lookup.unreflectSetter(Field) fails > to throw IllegalAccessException for fina

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

217 //If this is a USA test, then only the fraction of the expected failures will occur; those which are both static and final. 218 if (fl != FieldLookup.MH_UNREFLECT_SETTER_ACCESSIBLE && actualFieldNames.stream().anyMatch(s->!(s.contains("static")&&(s.contains("final") What is a USA test?

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

un against a non-patched java, so I think we're ok. Best Regards Adam Farley IBM Runtimes Mandy Chung wrote on 20/03/2019 05:08:37: > From: Mandy Chung > To: Adam Farley8 > Cc: core-libs-dev > Date: 20/03/2019 05:10 > Subject: Re: RFR: JDK-8216558: Lookup.unreflectSetter

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

Hi Adam, I imported your patch but there is one test failure: test/jdk/java/lang/invoke/VarHandles/accessibility/TestFieldLookupAccessibility.java This test needs update of this change. Can you please send an updated patch and run all test/jdk/java/lang/invoke tests. Mandy On 3/6/19 8:28

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

Mandy Chung , Adam Farley8 > > Cc: core-libs-dev > Date: 12/03/2019 03:34 > Subject: Re: RFR: JDK-8216558: Lookup.unreflectSetter(Field) fails > to throw IllegalAccessException for final fields > > > On 3/11/2019 3:49 PM, Mandy Chung wrote: > > > > > >

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

On 3/11/2019 3:49 PM, Mandy Chung wrote: On 3/11/19 8:48 AM, Adam Farley8 wrote: Hi Mandy, Thank you for explaining. :) Unfortunately I'm only a mere Author, and I cannot submit test runs on the shared test server. I have run the test locally, and it passes against a patched build and

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

: From: Mandy Chung To: Adam Farley8 Cc: core-libs-dev Date: 07/03/2019 23:19 Subject: Re: RFR: JDK-8216558: Lookup.unreflectSetter(Field) fails to throw IllegalAccessException for final fields On 3/7/19 9:44 AM, Adam Farley8 wrote: > Hi Mandy, > > Since you have created a new

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

: Adam Farley8 > Cc: core-libs-dev > Date: 07/03/2019 23:19 > Subject: Re: RFR: JDK-8216558: Lookup.unreflectSetter(Field) fails > to throw IllegalAccessException for final fields > > > > On 3/7/19 9:44 AM, Adam Farley8 wrote: > > Hi Mandy, > > > >

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

On 3/7/19 3:17 PM, Mandy Chung wrote: 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?

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

On 3/7/19 9:44 AM, Adam Farley8 wrote: Hi Mandy, 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 original issue. You push what you have. Your fix should come with a regression test. Please make sure

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

: Mandy Chung > To: Adam Farley8 > Cc: core-libs-dev > 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: > >

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: http://cr.openjdk.java.net/~afarley/8216558/webrev/ 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

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

gt; > > > Best Regards > > > > Adam Farley > > IBM Runtimes > > > > > > Mandy Chung wrote on 21/02/2019 17:37:54: > > > >> From: Mandy Chung > >> To: Adam Farley8 > >> Cc: core-libs-dev > >> Date: 21/02/2019 17:

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

ards Adam Farley IBM Runtimes Mandy Chung wrote on 21/02/2019 17:37:54: From: Mandy Chung To: Adam Farley8 Cc: core-libs-dev 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:1

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

es Mandy Chung wrote on 21/02/2019 17:37:54: > From: Mandy Chung > To: Adam Farley8 > Cc: core-libs-dev > Date: 21/02/2019 17:41 > Subject: Re: RFR: JDK-8216558: Lookup.unreflectSetter(Field) fails > to throw IllegalAccessException for final fields > > Hi Adam, >

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

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

Mandy Chung > To: Adam Farley8 > Cc: core-libs-dev > Date: 31/01/2019 18:58 > Subject: Re: RFR: JDK-8216558: Lookup.unreflectSetter(Field) fails > to throw IllegalAccessException for final fields > > > > On 1/31/19 4:18 AM, Adam Farley8 wrote: > > Hi Mandy, >

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

On 1/31/19 4:18 AM, Adam Farley8 wrote: Hi Mandy, I've made the changes to the webrev and uploaded the corrected version. I've also uploaded the zips in the csr and bug. As for the test, I have mentioned that the test is attached to the bug. I attached the same test to the CSR. I did

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

is not a suitable test? Please advise. Best Regards Adam Farley IBM Runtimes Mandy Chung wrote on 30/01/2019 18:22:40: > From: Mandy Chung > To: Adam Farley8 > Cc: core-libs-dev > Date: 30/01/2019 18:22 > Subject: Re: RFR: JDK-8216558: Lookup.unreflectSetter(Field) fa

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

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

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

on 23/01/2019 19:18:09: > From: Mandy Chung > To: Adam Farley8 > Cc: core-libs-dev > Date: 23/01/2019 19:17 > Subject: Re: RFR: JDK-8216558: Lookup.unreflectSetter(Field) fails > to throw IllegalAccessException for final fields > > Hi Adam, > > On 1/23/19 8:3

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

Hi Adam, On 1/23/19 8:33 AM, Adam Farley8 wrote: Hi Mandy, Food for thought: - From John's email, it looked like he was advocating a clarification change to the spec (which requires no CSR). While the new behavior was intended when the spec was written, this fix changes the current

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

2019 23:54 > Subject: Re: RFR: JDK-8216558: Lookup.unreflectSetter(Field) fails > to throw IllegalAccessException for final fields > > Hi Adam, > On 1/14/19 9:10 AM, Adam Farley8 wrote: > As for the CSR process, I'm unfamiliar with it. I've modified the > comment for the unrefl

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

Hi Adam, On 1/14/19 9:10 AM, Adam Farley8 wrote: 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. The CSR process is straight-forward.  CSR FAQ [1] may help you get started. As for

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

Hi John, On 15/01/2019 11:47 am, John Rose wrote: On Jan 11, 2019, at 4:53 PM, David Holmes > wrote: 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".

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

On Jan 11, 2019, at 4:53 PM, David Holmes wrote: > > 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

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

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

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

On 12/01/2019 10:13 am, Mandy Chung wrote: On 1/11/19 3:03 PM, David Holmes wrote: On 12/01/2019 8:47 am, Mandy Chung wrote: On 1/11/19 2:38 PM, David Holmes wrote: There seem to be a number of spec issues around this. Shouldn't findStaticSetter say something about what happens when the field

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

On 1/11/19 3:03 PM, David Holmes wrote: On 12/01/2019 8:47 am, Mandy Chung wrote: On 1/11/19 2:38 PM, David Holmes wrote: There seem to be a number of spec issues around this. Shouldn't findStaticSetter say something about what happens when the field is final? Same for findSetter? This

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

On 11/01/2019 11:11 pm, Remi Forax wrote: - Mail original - De: "David Holmes" À: "Adam Farley8" , "core-libs-dev" Envoyé: Vendredi 11 Janvier 2019 14:02:06 Objet: Re: RFR: JDK-8216558: Lookup.unreflectSetter(Field) fails to throw IllegalAc

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

On 12/01/2019 8:47 am, Mandy Chung wrote: On 1/11/19 2:38 PM, David Holmes wrote: There seem to be a number of spec issues around this. Shouldn't findStaticSetter say something about what happens when the field is final? Same for findSetter? This issue seems to be much bigger than just a

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

On 1/11/19 2:38 PM, David Holmes wrote: There seem to be a number of spec issues around this. Shouldn't findStaticSetter say something about what happens when the field is final? Same for findSetter? This issue seems to be much bigger than just a simple bug fix. I don't see any issue

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

There seem to be a number of spec issues around this. Shouldn't findStaticSetter say something about what happens when the field is final? Same for findSetter? This issue seems to be much bigger than just a simple bug fix. A CSR request will need to be filed. David - On 12/01/2019 5:07

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

This issue requires a spec change and it's a behavioral change. Field::set on a static final field throws IAE.  Although there are few cases in the world that hack around it, I think the compatibility risk is low in this change. I added a suggested fix in the JBS issue. Mandy On 1/11/19

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

Hi Adam, This is indeed a bug.    I suggest to do the check in the checkAccess method closes to where it performs the instance final field check.  Please also add a test case. Mandy On 1/11/19 6:38 AM, Adam Farley8 wrote: Hi All, First; thanks for responding so quickly. :) Second, it

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

.java.net> > Date: 11/01/2019 13:11 > Subject: Re: RFR: JDK-8216558: Lookup.unreflectSetter(Field) fails > to throw IllegalAccessException for final fields > > > > - Mail original - > > De: "David Holmes" > > À: "Adam Farley8" , &quo

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

- Mail original - > De: "David Holmes" > À: "Adam Farley8" , "core-libs-dev" > > Envoyé: Vendredi 11 Janvier 2019 14:02:06 > Objet: Re: RFR: JDK-8216558: Lookup.unreflectSetter(Field) fails to throw > IllegalAccessException for fina

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

On 11/01/2019 10:46 pm, Adam Farley8 wrote: Hi All, I posit that you shouldn't be able to change the contents of a a final field. Nice in theory, but not true in practice. However, if you use Field.setAccessible(true) before calling Lookup.unreflectSetter(Field), then you can get a

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

(field).invokeExact(object, value) has to change the value of the field too. regards, Rémi (with my JSR 292 Expert hat) - Mail original - > De: "Adam Farley8" > À: "core-libs-dev" > Envoyé: Vendredi 11 Janvier 2019 13:46:01 > Objet: RFR: JDK-8216558: Loo

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

Hi All, I posit that you shouldn't be able to change the contents of a a final field. However, if you use Field.setAccessible(true) before calling Lookup.unreflectSetter(Field), then you can get a MethodHandle that allows you to set the (reflected) value of a static final field. This seems