RFR: 8200145: Conditional expression mistakenly treated as standalone

[Jan Lahoda]

On Fri, 16 Apr 2021 15:53:34 GMT, Maurizio Cimadamore <mcimadamore 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
>
> 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.

(I am not an expert on types, so I may be wrong.)

This method seems to return either the primitive type for which `t` is a box, or `noType`. So returning a `noType` for erroneous type seems reasonable to me. Basically, it would mean that an erroneous type is not a box for any type, which seems sensible. (Currently, every erroneous type is seen as the box for the first primitive type in the sequence, which I think is `byte`.)

I agree only one of these two changes is needed, although I'd personally probably try to go with the one here, in `unboxedType`, as it seems like a generally desirable behavior to me. I agree it can have effects on other parts of the code, though, so it has more potential to break something.

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

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