Records, Intersection type and lambda

[Vicente Romero]

Hi Bernard,

On 1/6/21 8:02 AM, B. Blaser wrote:
> Hi Vicente,
>
> I saw you've integrated the initial fix with no additional change
> which seems reasonable.
>
> However, did you also try the example I provided below

no I didn't sorry, I think that we will have to file a follow-up issue
>   as I'm not
> absolutely sure this works?
> Otherwise, you may look at the experimental fix I suggested.
>
> Thanks,
> Bernard

Thanks,
Vicente

>
> On Sun, 27 Dec 2020 at 22:31, B. Blaser <bsrbnd at gmail.com> wrote:
>> Hi Vicente,
>>
>> On Wed, 23 Dec 2020 at 04:29, Vicente Romero <vicente.romero at oracle.com> wrote:
>>> Hi Bernard,
>>>
>>> what side effects do you see?
>> By tweaking Remi's example:
>>
>> public class RecLCE {
>>    interface Foo { }
>>    interface F<T extends Foo> {
>>      void call(T t);
>>    }
>>
>>    record Bar1() implements Foo { }
>>    record Bar2() implements Foo { }
>>    record Pair<P extends Foo>(P p1, P p2) {
>>
>>      static <P extends Foo> Pair<P> of(P p1, P p2) {
>>        return new Pair<P>(p1, p2);
>>      }
>>
>>      void forEach(F<P> f) {
>>        f.call(p1);
>>        f.call(p2);
>>      }
>>    }
>>
>>    static class Hello {
>>      void m(Foo foo) {
>>        System.out.println(foo.getClass());
>>      }
>>    }
>>
>>    public static void main(String[] args) {
>>      var pair = Pair.of(new Bar1(), new Bar2());
>>      pair.forEach(new Hello()::m);
>>    }
>> }
>>
>> The line you've added is causing the lambda to use the instantiated
>> type 'Record' which isn't a subtype of 'Foo', see javap's output:
>>      Method arguments:
>>        #57 (LRecLCE$Foo;)V
>>        #58 REF_invokeStatic
>> RecLCE.lambda$main$0:(LRecLCE$Hello;Ljava/lang/Record;)V
>>        #61 (Ljava/lang/Record;)V
>>
>> So, it should use the un-instantiated SAM parameter type 'Foo' instead
>> which my initial fix was suggesting:
>>      Method arguments:
>>        #57 (LRecLCE$Foo;)V
>>        #58 REF_invokeStatic RecLCE.lambda$main$0:(LRecLCE$Hello;LRecLCE$Foo;)V
>>        #61 (Ljava/lang/Record;)V
>>
>> Unfortunately, this very example reveals another issue as the
>> instantiated type 'Record' is still not convertible to 'Foo' although
>> all was working fine with Remis's initial example:
>>      Method arguments:
>>        #55 (Ljava/lang/Object;)V
>>        #57 REF_invokeStatic
>> RecordIntersectionTypeAndLambda.lambda$main$0:(LRecordIntersectionTypeAndLambda$Hello;Ljava/lang/Object;)V
>>        #60 (Ljava/lang/Record;)V
>>
>> Referring to 'LambdaMetafactory::metafactory', we see that
>> 'instantiatedMethodType' may be the same or a specialization of
>> 'samMethodType' suggesting to use the SAM type too when the
>> instantiated type is compound like in the experimental fix below
>> (langtools:tier1 is OK on jdk14u):
>>      Method arguments:
>>        #57 (LRecLCE$Foo;)V
>>        #58 REF_invokeStatic RecLCE.lambda$main$0:(LRecLCE$Hello;LRecLCE$Foo;)V
>>        #57 (LRecLCE$Foo;)V
>>
>> What do you think?
>> Bernard
>>
>> diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/LambdaToMethod.java
>> b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/LambdaToMethod.java
>> --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/LambdaToMethod.java
>> +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/LambdaToMethod.java
>> @@ -438,6 +438,7 @@
>>           List<JCExpression> indy_args =
>> translate(syntheticInits.toList(), localContext.prev);
>>
>>           //convert to an invokedynamic call
>> +        localContext.useSAM = tree.useSAM;
>>           result = makeMetafactoryIndyCall(context, sym.asHandle(), indy_args);
>>       }
>>
>> @@ -888,6 +889,7 @@
>>           private final ListBuffer<JCVariableDecl> params = new ListBuffer<>();
>>
>>           private JCExpression receiverExpression = null;
>> +        private boolean useSAM = false;
>>
>>           MemberReferenceToLambda(JCMemberReference tree,
>> ReferenceTranslationContext localContext, Symbol owner) {
>>               this.tree = tree;
>> @@ -911,6 +913,7 @@
>>                   slam.target = tree.target;
>>                   slam.type = tree.type;
>>                   slam.pos = tree.pos;
>> +                slam.useSAM = useSAM;
>>                   return slam;
>>               } finally {
>>                   make.at(prevPos);
>> @@ -954,6 +957,7 @@
>>
>>               // Failsafe -- assure match-up
>>               boolean checkForIntersection = tree.varargsElement !=
>> null || implSize == descPTypes.size();
>> +            useSAM = checkForIntersection &&
>> localContext.interfaceParameterIsIntersectionOrUnionType();
>>
>>               // Use parameter types of the implementation method
>> unless the unerased
>>               // SAM parameter type is an intersection type, in that case use the
>> @@ -963,18 +967,7 @@
>>               // are used as pointers to the current parameter type information
>>               // and are thus not usable afterwards.
>>               for (int i = 0; implPTypes.nonEmpty() && i < last; ++i) {
>> -                // By default use the implementation method parmeter type
>> -                Type parmType = implPTypes.head;
>> -                // If the unerased parameter type is a type variable whose
>> -                // bound is an intersection (eg. <T extends A & B>) then
>> -                // use the SAM parameter type
>> -                if (checkForIntersection && descPTypes.head.getKind()
>> == TypeKind.TYPEVAR) {
>> -                    TypeVar tv = (TypeVar) descPTypes.head;
>> -                    if (tv.getUpperBound().getKind() ==
>> TypeKind.INTERSECTION) {
>> -                        parmType = samPTypes.head;
>> -                    }
>> -                }
>> -                addParameter("x$" + i, parmType, true);
>> +                addParameter("x$" + i, useSAM ? samPTypes.head :
>> implPTypes.head, true);
>>
>>                   // Advance to the next parameter
>>                   implPTypes = implPTypes.tail;
>> @@ -1094,7 +1087,7 @@
>>           List<LoadableConstant> staticArgs = List.of(
>>                   typeToMethodType(samSym.type),
>>                   refSym.asHandle(),
>> -                typeToMethodType(tree.getDescriptorType(types)));
>> +                typeToMethodType(context.useSAM ? samSym.type :
>> tree.getDescriptorType(types)));
>>
>>           //computed indy arg types
>>           ListBuffer<Type> indy_args_types = new ListBuffer<>();
>> @@ -1826,6 +1819,8 @@
>>               /** list of methods to be bridged by the meta-factory */
>>               final List<Symbol> bridges;
>>
>> +            boolean useSAM = false;
>> +
>>               TranslationContext(T tree) {
>>                   this.tree = tree;
>>                   this.owner = owner(true);
>> diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/JCTree.java
>> b/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/JCTree.java
>> --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/JCTree.java
>> +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/JCTree.java
>> @@ -1892,6 +1892,7 @@
>>           public JCTree body;
>>           public boolean canCompleteNormally = true;
>>           public ParameterKind paramKind;
>> +        public boolean useSAM = false;
>>
>>           public JCLambda(List<JCVariableDecl> params,
>>                           JCTree body) {