RFR: 8232765: NullPointerException at Types.eraseNotNeeded() when compiling a class [v2]

[Maurizio Cimadamore]

On Fri, 23 Apr 2021 16:33:13 GMT, Guoxiong Li <gli at openjdk.org> wrote:

>> Hi all,
>> 
>> The method `types::asSuper` may return `null` so that `types.erasure(types.asSuper(iterator.type.getReturnType(), syms.iteratorType.tsym))` throws `NPE`.
>> 
>> This patch sets the return type and outputs the corresponding error message if `types::asSuper` returns `null`.
>> 
>> Thank you for taking the time to review.
>> 
>> Best Regards.
>
> Guoxiong Li has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:
> 
>  - Move the check to Attr. Use meaningful class name. Fix failed tests.
>  - Merge branch 'master' into JDK-8232765
>  - 8232765: NullPointerException at Types.eraseNotNeeded() when compiling a class

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java line 1545:

> 1543:                     if (types.asSuper(iterSymbol.type.getReturnType(), syms.iteratorType.tsym) == null) {
> 1544:                         log.error(tree.pos(),
> 1545:                                 Errors.OverrideIncompatibleRet(

I believe here we don't need a new diagnostic - let's just reuse:


log.error(tree.expr.pos(),
                              Errors.ForeachNotApplicableToType(exprType,
                                                                Fragments.TypeReqArrayOrIterable));


On the basis that the provided type is NOT an well-formed `Iterable` type (because it has a bogus override).

The thing I'm a bit afraid, if we go down this path is that we will need to replicate all the override checks here - what if the method has the right signature but is private? It's still a bogus override.

I could be fine if the answer was "this is the bare minimum we need to verify to make sure code generation doesn't crash". But I'd like to see a comment on the code stating exactly that.

Do we have a similar problem with AutoCloseable? E.g. a close() method returning something odd?

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

PR: https://git.openjdk.java.net/jdk/pull/2099