hg: valhalla/valhalla: 8198748: [lworld] Javac should insert null checks at casts to value types.

[Srikanth]

(Code indentation below got messed up automatically - sorry about that)

On Monday 12 March 2018 01:16 PM, Srikanth wrote:
>
>
> On Monday 12 March 2018 12:29 PM, Remi Forax wrote:
>> Hi Srikanth,
>> thanks for doing that.
>>
>> There is already a code path in the compiler to generate nullchecks 
>> using getClass() + pop,
>> i think you should reuse this code instead of adding another way to 
>> do a nullcheck.
> Hello Remi,
>
> Javac *already* employs two distinct mechanisms to implement null checks:
>
> Please see: Gen#genNullCheck
>
> /** Generate a null check from the object value at stack top. */ 
> private void genNullCheck(JCTree tree) {
>     code.statBegin(tree.pos); if (allowBetterNullChecks) {
>         callMethod(tree.pos(), syms.objectsType, names.requireNonNull, 
> List.of(syms.objectType), true); }else {
>         callMethod(tree.pos(), syms.objectType, names.getClass, 
> List.nil(), false); }
>     code.emitop0(pop); }
>
>
> where the controlling boolean is initialized as:
>
> allowBetterNullChecks =target.hasObjects();
>
>
> and target.hasObjects() is really:
>
> /** Does the target JDK contains the java.util.Objects class? */ 
> public boolean hasObjects() {
>     return compareTo(JDK1_7) >=0; }
>
>
> In short, I am not expressly selecting the method of implementing the 
> null check, only inserting a null-check AST node.
>
> So this pattern is already emitted by javac and any/all analyzers 
> should anyway be upgraded (should have been upgraded already)
>
> Thanks!
> Srikanth
>
>> And with my hat of backporter/bytecode analyzer writer, adding a new 
>> runtime dependency, here, to Objects.requireNonNull, makes things 
>> more complex because some analyzers may have to be enhanced to 
>> recognize this new pattern.
>>
>> cheers,
>> Rémi
>>
>> ----- Mail original -----
>>> De: "Srikanth" <srikanth.adayapalam at oracle.com>
>>> À: "valhalla-dev" <valhalla-dev at openjdk.java.net>
>>> Envoyé: Lundi 12 Mars 2018 07:32:13
>>> Objet: Re: hg: valhalla/valhalla: 8198748: [lworld] Javac should 
>>> insert null checks at casts to value types.
>>> On Monday 12 March 2018 11:51 AM, srikanth.adayapalam at oracle.com wrote:
>>>> Changeset: 7c1c51c340e6
>>>> Author:    sadayapalam
>>>> Date:      2018-03-12 11:46 +0530
>>>> URL: http://hg.openjdk.java.net/valhalla/valhalla/rev/7c1c51c340e6
>>>>
>>>> 8198748: [lworld] Javac should insert null checks at casts to value 
>>>> types.
>>> Notes:
>>>
>>> Where Point is a value type, code generated for casting an object to
>>> Point as in:
>>>
>>> Object o = ...
>>>
>>> Point p = (Point) o;
>>>
>>> is:
>>>
>>>           2: aload_1
>>>           3: dup
>>>           4: invokestatic  #2                  // Method
>>> java/util/Objects.requireNonNull:(Ljava/lang/Object;)Ljava/lang/Object;
>>>           7: pop
>>>           8: checkcast     #3                  // class Point
>>>          11: astore_2
>>>
>>> If the cast occurs in source code compiled at a level where value types
>>> are not recognized as such, then there is no insertion of null check.
>>>
>>> (FWIW, Javac is not in the business of nullness tracking, so these
>>> checks will be inserted even where a cursory glance of source code
>>> context could inform a reader that no null check would be needed)
>>>
>>> Thanks
>>> Srikanth
>>>
>>>> ! src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java
>>>> ! 
>>>> src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java 
>>>>
>>>> + 
>>>> test/langtools/tools/javac/valhalla/lworld-values/CastNoNullCheckTest.java
>>>> + 
>>>> test/langtools/tools/javac/valhalla/lworld-values/CastNullCheckTest.java 
>>>>
>