RFR: 8200145: Conditional expression mistakenly treated as standalone

[Maurizio Cimadamore]

On Sat, 30 Jan 2021 08:50:53 GMT, Guoxiong Li <github.com+13688759+lgxbslgx at openjdk.org> wrote:

> Hi all,
> 
> If the argument `Type t` of the method `Types.unboxedType` is an `ErrorType`, the `Types.unboxedType` may return the wrong result. And in this case, `Attr.isBooleanOrNumeric` and `Attr.isBooleanOrNumeric` return the wrong result, too.
> 
> This patch fixes it and adds a test case.
> Thank you for taking the time to review.
> 
> Best Regards.
> -- xiong

The fix looks good - it's a good "quality of life" fix, which improves the conditional classification by making it more resilient to erroneous types. I wonder if the fix can be made more minimal (see code comment).

src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java line 4326:

> 4324:      */
> 4325:     public Type unboxedType(Type t) {
> 4326:         if (t.hasTag(ERROR))

This is not necessary, right? After all, you check for errors upfront, in the conditional code. Reason I'm asking is that it's not clear to me as to whether we should just return the error type, or no type. Also, this might affect code which is unrelated to the one you are fixing.

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

> 1957:         //where
> 1958:             boolean primitiveOrBoxed(Type t) {
> 1959:                 return (!t.hasTag(TYPEVAR) && !t.hasTag(ERROR) && types.unboxedTypeOrType(t).isPrimitive());

Good catch - you can also use `!t.isErroneous()` here.

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

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