RFR: 8303755: Clean up around JavacElements.getAllMembers [v3]

[Jan Lahoda]

On Tue, 7 Mar 2023 19:52:12 GMT, Pavel Rappo <prappo at openjdk.org> wrote:

>> Please review this cleanup which consists of commentary changes and code simplifications.
>
> Pavel Rappo has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Respond to feedback

Overall, looks fine to me. I've left some inline comments in Iterators.

src/jdk.compiler/share/classes/com/sun/tools/javac/util/Iterators.java line 47:

> 45:     }
> 46: 
> 47:     private static final class CompoundIterator<I, O> implements Iterator<O> {

Nit: why final? Seems quite unnecessary for a private class?

src/jdk.compiler/share/classes/com/sun/tools/javac/util/Iterators.java line 68:

> 66:         @Override
> 67:         public O next() {
> 68:             if (!hasNext()) {

I was looking into the history, and the reason for the current code is JDK-8167070 (performance). So, I'd suggest to try how this new code performs. The pre-JDK-8167070 did two calls to nested Iterator's hasNext for each call to hasNext (so for deeper hierarchies, the number of invocations grew very fast), while the code here only does one, so it quite probably may be OK.

I wonder if it is time to write a (unit) test for this class?

src/jdk.compiler/share/classes/com/sun/tools/javac/util/Iterators.java line 83:

> 81:     }
> 82: 
> 83:     // Iterator is assumed to never return null from next()

Not sure about this comment - it seems to explain what is wrong in the code that is removed?

-------------

PR: https://git.openjdk.org/jdk/pull/12904