On Tue, 30 Sep 2025 15:06:11 GMT, Alan Bateman <[email protected]> wrote:
>> I've read the JEP, and reviewed the tests. I hope I manage to contribute >> meaningfully. >> >> A common theme I've struggled with: While JEP explicitly mentions of >> `static` fields, and `MethodHandle`s, they are not always exercised in >> tests. It very well might be unnecessary given the context, but dropped some >> comments anyway. >> >> For those who were about to jump the gun after seeing no mentions of >> `VarHandle`s: _"The 2 APIs that are changed by the JEP are `Field.setXXX` >> and `Lookup.unreflectSetter`. A `VarHandle` produced by >> `Lookup.unreflectVarHandle` has never been allowed write access to final >> fields, so no change there, `UOE` will continue to be thrown."_ – >> @AlanBateman > >> I've read the JEP, and reviewed the tests. I hope I manage to contribute >> meaningfully. > > @vy Thank you very much for your thorough review and walk through of all of > the tests. You clearly spent a lot of time on this. I went through your > comments and pushed an update [5acb1d727c5e0feee52c7a1f47665264eb534489]( > https://github.com/openjdk/jdk/pull/25115/commits/5acb1d727c5e0feee52c7a1f47665264eb534489) > to add more tests, improves some comments, and remove the unused setInt > methods that you spotted in the x-module test. Many of the tests are focused > on specific parts of the solution (APIs, access checks, CLI options, JAR file > manifest, JNI, ...) and I've resisted going further with some of these tests > to avoid too much overlap. You have rightly identified a few opportunities > for more tests, e.g. JNI attached thread doing upcall to mutate a final field > in named module as the tests only checks the unnamed module case right now. > This are good suggestions but they require a good bit of setup, I'll try to > get to it (or find someone) so that we have more tests before we are done. @AlanBateman - Sorry should have realized this sooner. This is only the preparatory step so writing to final fields is still allowed at the moment, in which case -Xcheck:jni should only be issuing a warning, otherwise code that still writes to finals (but which knows it has to stop before final-really-means-final) won't be able to use -Xcheck:jni to watch for other problems. ------------- PR Comment: https://git.openjdk.org/jdk/pull/25115#issuecomment-3368748448
