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 included a handful of small improvements, and that the final
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:
   https://bugs.openjdk.java.net/browse/JDK-8220282

FYI.  I have a patch for it and will send out for review.
  http://cr.openjdk.java.net/~mchung/jdk13/webrevs/8220282/webrev.00/

Mandy

Reply via email to