Records, Intersection type and lambda

[B. Blaser]

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 as I'm not
absolutely sure this works?
Otherwise, you may look at the experimental fix I suggested.

Thanks,
Bernard

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