JDK-8179483: Return type inference in if

[Johannes Kuhn]

On 30-Jun-22 17:12, forax at univ-mlv.fr wrote:
> ----- Original Message -----
>> From: "Johannes Kuhn" <info at j-kuhn.de>
>> To: "Remi Forax" <forax at univ-mlv.fr>
>> Cc: "classfile-api-dev" <classfile-api-dev at openjdk.org>
>> Sent: Thursday, June 30, 2022 4:44:37 PM
>> Subject: Re: JDK-8179483: Return type inference in if
> 
>> On 30-Jun-22 16:32, Remi Forax wrote:
>>> ----- Original Message -----
>>>> From: "Johannes Kuhn" <info at j-kuhn.de>
>>>> To: classfile-api-dev at openjdk.org
>>>> Sent: Thursday, June 30, 2022 3:31:57 PM
>>>> Subject: JDK-8179483: Return type inference in if
>>>
>>>> So, I tried to extract the jdk.classfile API and compile it using Java
>>>> 18 with preview features enabled. Using Eclipse.
>>>>
>>>> While doing that, I encountered some compilation problems - one is the
>>>> use of something similar like this:
>>>>
>>>>       <T> T optionValue(Classfile.Option.Key key) {...}
>>>>       ...
>>>>       if (reader.optionValue(...)) {...}
>>>>
>>>> According to https://bugs.openjdk.org/browse/JDK-8179483 ('if'
>>>> conditions are treated like assignments by inference) this abuses a bug
>>>> in javac - and is not valid java.
>>>>
>>>> The places where I found a use like this are:
>>>>
>>>> https://github.com/openjdk/jdk-sandbox/blob/classfile-api-branch/src/java.base/share/classes/jdk/classfile/constantpool/ConstantPoolBuilder.java#L76
>>>> https://github.com/openjdk/jdk-sandbox/blob/classfile-api-branch/src/java.base/share/classes/jdk/classfile/impl/BoundAttribute.java#L150
>>>> https://github.com/openjdk/jdk-sandbox/blob/classfile-api-branch/src/java.base/share/classes/jdk/classfile/impl/CodeImpl.java#L153
>>>> https://github.com/openjdk/jdk-sandbox/blob/classfile-api-branch/src/java.base/share/classes/jdk/classfile/impl/CodeImpl.java#L208
>>>> https://github.com/openjdk/jdk-sandbox/blob/classfile-api-branch/src/java.base/share/classes/jdk/classfile/impl/DirectCodeBuilder.java#L111
>>>> https://github.com/openjdk/jdk-sandbox/blob/classfile-api-branch/src/java.base/share/classes/jdk/classfile/impl/DirectCodeBuilder.java#L209
>>>> https://github.com/openjdk/jdk-sandbox/blob/classfile-api-branch/src/java.base/share/classes/jdk/classfile/impl/DirectCodeBuilder.java#L277
>>>
>>> yes, this is an annoying bug of javac, i've also encounter it in student codes
>>> :)
>>>
>>> Here, the real question is why optionValue() has such weird signature, it means
>>> there is a @SuppressWarnings("unchecked") which is not safe in the
>>> implementation.
>>> This kind of signature is only valid if T is always null, like in
>>> Collections.emptyList().
>>>
>>
>>  From some point of view, it provides a "nice" API - if it would return
>> Object - as it should - then a cast is needed at the callsite. Which is
>> IMHO not that bad, but preferences differ, and some people prefer to
>> write clever code.
> 
> It's clever until it's not, by example
>    var list = new ArrayList<SpaceGiant>();
>    list.add(optionValue(...));
> 
> the compiler will be happy to compile that snippet and *not* insert a cast because optionValue() says it returns a space giant and both E and T erases to Object.

That... sounds wrong. It should. Javac 17 does.
https://gist.github.com/DasBrain/212f36f88978b7ddd4fb6ea3a97a392e (fails 
with CCE)
Might be a different story if it's an ArrayList<T> (T is a type variable).

> 
>>
>> I have to admit, the use of such a method is nice - but IMHO such a
>> method can only ever fulfill its contract (return something the caller
>> wants, but the caller doesn't tell what it wants) if it returns `null`.
> 
> This is a kind of okay if you are both the guy that write optionValue() and call it, otherwise it's a good way to waste people time and awake space giants :)

Except that optionValue() is a public method:
https://htmlpreview.github.io/?https://raw.githubusercontent.com/openjdk/jdk-sandbox/classfile-api-javadoc-branch/doc/classfile-api/javadoc/jdk/classfile/ClassReader.html#optionValue(jdk.classfile.Classfile.Option.Key)

(I didn't check _that_ before my initial report.)

- Johannes