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

[Jonathan Gibbons]

On Tue, 7 Mar 2023 16:37:33 GMT, Pavel Rappo <prappo at openjdk.org> wrote:

>> src/jdk.compiler/share/classes/com/sun/tools/javac/util/Iterators.java line 89:
>> 
>>> 87:                 while (input.hasNext()) {
>>> 88:                     E sym = input.next();
>>> 89:                     assert sym != null; // otherwise, false positive EOF
>> 
>> javac coding style is to use the `Assert` class, not `assert` statement
>
> If you are considering an unconditional null check here, then you might want to consider a dedicated EOF flag instead. Alternatively, I can remove the `assert`, but leave the comment; for the next reader.
> 
> It's probably the case that iteration elements are never `null`. If they were `null`, we would either notice false positive EOF or see NPEs thrown from predicates.

You were the one to add the `assert` ;-) 
I just made a statement that overall, the coding style in javac has been to use `Assert` instead of `assert`, at least in those cases where it is appropriate to make an assertion.
Looking at the code in more detail, it's not clear to me that either an assertion or a comment (here) is required. If you want a comment, I'd suggest a more declarative comment on the class itself.

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

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