JDK-8179483: Return type inference in if

[forax at univ-mlv.fr]

----- 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.

> 
> 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 :)

Rémi