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 look at that but it's not a jtreg regression test. I assume
you planned to convert it to jtreg test. Anyway, the test attached
in the bug report is the reproducer of the bug.
It would help the reviewer to include the regression test in
the webrev along with the fix unless a test is not necessary.
The OpenJDK developer guide gives a pretty good overview on the
process [1] and please do ask if you have any question.
http://openjdk.java.net/guide/changePlanning.html#bug
Are you saying that the test supplied is not a suitable test?
TestStaticFinal.java is a standalone test. It needs to convert
to a jtreg regression test.
The regression should throw an exception if Lookup::unreflectSetter
returns MH on a static final field with and without accessible flag
set. However TestStaticFinal.java also exits with 0. The regression
test should pass with your bug and fail without the fix (throwing
an exception).
This test verifies unreflectSetter on a static final field. I
suggest it also includes a case with instance final field with
and without accessible flag set to show the exception for
unreflectSetter what case allows setting final fields.
Mandy
[1] http://openjdk.java.net/guide/changePlanning.html#bug
P.S. should be updated to refer to CSR that replaced CCC.
