On Fri, 22 Oct 2021 10:17:13 GMT, Alan Bateman <[email protected]> wrote:
>> Jaikiran Pai has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - drop final >> - no need to check for null values > > src/java.base/share/classes/java/lang/module/ModuleDescriptor.java line 2559: > >> 2557: continue; >> 2558: } >> 2559: h += e.ordinal(); > > This e == null check suggests there is an issue elsewhere, can you explain > the scenario where you see this? Also can you drop the spurious use of > "final" here, it's inconsistent with the existing code. > > Do you want us to look at the test? It looks like it needs clean-up and I'm > not sure if you want to wait for the CDS issue or not. Hello Alan, > This e == null check suggests there is an issue elsewhere, can you explain > the scenario where you see this? That was just a pre-emptive defensive check I had added. There isn't a scenario in that flow where any of the elements are currently null. I have updated this PR to remove that check. Test continues to pass with this change. Also removed the "final". > Do you want us to look at the test? It looks like it needs clean-up and I'm > not sure if you want to wait for the CDS issue or not. Yes please. The test is ready for review. The only place where there's a `TODO` is where that part of commented code is affected by the bug mentioned in that comment. I would like to use that check (the commented out one) but I don't want to wait for that other bug to be fixed for this PR, since I am unsure how big or how long it might take for the other bug to be fixed. I plan to uncomment that part in a separate PR once that other bug is fixed, if that's OK. ------------- PR: https://git.openjdk.java.net/jdk/pull/6078
