RFR: JDK-8184989. Incorrect class file created when passing lambda in inner class constructor and outer is subclass

B. Blaser bsrbnd at gmail.com
Thu Aug 3 21:32:06 UTC 2017


Looking closer at this patch, I think that simply looping on the
key-set could fail in some cases.
Consider the following example:

class A {
    public boolean test(){
        return true;
    }
    class AA{
        public AA(Condition<AA> condition) {
            System.out.println("? " + condition.check(this));
        }
    }
}

interface Condition<T> {
    boolean check(T t);
}

public class B extends A {
    public boolean test() {return false;}
    public void b() {}

    class C extends A {
        public class BA extends AA {
            public BA() {
                super(o -> {b(); return test();});
            }
        }
    }
    public static void main(String[] args) {
        new B().new C().new BA();
    }
}

The invocation of b() implies a proxy to B and the invocation of
test() involves a proxy to C.
But the simple loop on the key-set chooses the first proxy to B for
the invocation of test() and our example prints "false" instead of
"true".

I suggest the following improvement of the loop that chooses the
innermost proxy.
What do you think of that?

Thanks,
Bernard

diff -r 43a83431f19d
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
   Wed Mar 15 15:46:43 2017 +0100
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/LambdaToMethod.java
   Thu Aug 03 23:03:08 2017 +0200
@@ -2103,9 +2103,22 @@
                             }
                             break;
                         case CAPTURED_OUTER_THIS:
-                            if (lambdaIdent.sym.owner.kind == TYP &&
m.containsKey(lambdaIdent.sym.owner)) {
+                            ClassSymbol proxy = null;
+                            if (lambdaIdent.sym.owner.kind == TYP) {
+                                for (Symbol out: m.keySet()) {
+                                    for (Type type = out.type;
+                                            type.hasTag(CLASS) &&
!type.isErroneous();
+
type=((ClassSymbol)type.tsym).getSuperclass()) {
+                                        if
(type.tsym.equals(lambdaIdent.sym.owner)
+                                                && (proxy == null ||
out.isEnclosedBy(proxy))) {
+                                            proxy = (ClassSymbol)out;
+                                        }
+                                    }
+                                }
+                            }
+                            if (proxy != null) {
                                 // Transform outer instance variable
references anchoring them to the captured synthetic.
-                                Symbol tSym = m.get(lambdaIdent.sym.owner);
+                                Symbol tSym = m.get(proxy);
                                 JCExpression t =
make.Ident(tSym).setType(lambdaIdent.sym.owner.type);

tSym.setTypeAttributes(lambdaIdent.sym.owner.getRawTypeAttributes());
                                 t = make.Select(t, lambdaIdent.name);


On 2 August 2017 at 15:02, B. Blaser <bsrbnd at gmail.com> wrote:
> Andrey, Srikanth, All,
>
> At first sight, this looks good to me.
>
> I would perhaps rewrite it as here under, to be more expressive.
> I did it rapidly on a quite old jdk9 revision (sorry...), but this
> should work on the latest, too.
>
> What do you think of this rewriting?
>
> Regards,
> Bernard
>
> diff -r 43a83431f19d
> 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
>    Wed Mar 15 15:46:43 2017 +0100
> +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/LambdaToMethod.java
>    Wed Aug 02 14:11:08 2017 +0200
> @@ -2103,15 +2103,23 @@
>                              }
>                              break;
>                          case CAPTURED_OUTER_THIS:
> -                            if (lambdaIdent.sym.owner.kind == TYP &&
> m.containsKey(lambdaIdent.sym.owner)) {
> -                                // Transform outer instance variable
> references anchoring them to the captured synthetic.
> -                                Symbol tSym = m.get(lambdaIdent.sym.owner);
> -                                JCExpression t =
> make.Ident(tSym).setType(lambdaIdent.sym.owner.type);
> -
> tSym.setTypeAttributes(lambdaIdent.sym.owner.getRawTypeAttributes());
> -                                t = make.Select(t, lambdaIdent.name);
> -                                t.setType(lambdaIdent.type);
> -                                TreeInfo.setSymbol(t, lambdaIdent.sym);
> -                                return t;
> +                            if (lambdaIdent.sym.owner.kind == TYP) {
> +                                for (Symbol out: m.keySet()) {
> +                                    for (Type type = out.type;
> +                                            type.hasTag(CLASS) &&
> !type.isErroneous();
> +
> type=((ClassSymbol)type.tsym).getSuperclass()) {
> +                                        if
> (type.tsym.equals(lambdaIdent.sym.owner)) {
> +                                            // Transform outer
> instance variable references anchoring them to the captured synthetic.
> +                                            Symbol tSym = m.get(out);
> +                                            JCExpression t =
> make.Ident(tSym).setType(lambdaIdent.sym.owner.type);
> +
> tSym.setTypeAttributes(lambdaIdent.sym.owner.getRawTypeAttributes());
> +                                            t = make.Select(t,
> lambdaIdent.name);
> +                                            t.setType(lambdaIdent.type);
> +                                            TreeInfo.setSymbol(t,
> lambdaIdent.sym);
> +                                            return t;
> +                                        }
> +                                    }
> +                                }
>                              }
>                              break;
>                      }
>
>
> On 1 August 2017 at 14:15, Srikanth <srikanth.adayapalam at oracle.com> wrote:
>>
>> Thanks Andrey.
>>
>> I'll take a look in the coming week and let you know of any feedback.
>>
>> Srikanth
>>
>>
>> On Monday 31 July 2017 08:00 PM, Andrey Petushkov wrote:
>>
>> Dear Compiler Team,
>>
>> We’ve occasionally found out that there is a problem in javac related to
>> lambda support, in part related to referencing the outer’s entity which
>> otherwise would be captured by using inner this, but cannot be since it’s
>> not yet fully initialized. More specifically this seem to be omission in the
>> fix for JDK-8129740:
>> it's fix works fine if it deals with the entity from the outer class(es),
>> but fails if the entity is inherited from outer’s parent. Consider the
>> following source code:
>>
>> A.java
>> public class A {
>>     public boolean test(){
>>         return true;
>>     }
>>     class AA{
>>         public AA(Condition condition) {
>>         }
>>     }
>> }
>>
>> Condition.java
>> public interface Condition<T> {
>>     boolean check(T t);
>> }
>>
>> B.java
>> public class B extends A {
>>     private final BA myBA;
>>     public B() {
>>         myBA = new BA();
>>     }
>>     public class BA extends AA{
>>         public BA() {
>>             super(o -> test());
>>         }
>>     }
>>     public static void main(String[] args) {
>>         B b = new B();
>>         System.out.println(b);
>>     }
>> }
>>
>>
>> source compiles but execution of B fails with:
>> Exception in thread "main" java.lang.VerifyError: Bad type on operand stack
>> Exception Details:
>>   Location:
>>     B$BA.lambda$new$0(LB;Ljava/lang/Object;)Z @1: getfield
>>   Reason:
>>     Type 'B' (current frame, stack[0]) is not assignable to 'B$BA'
>>   Current Frame:
>>     bci: @1
>>     flags: { }
>>     locals: { 'B', 'java/lang/Object' }
>>     stack: { 'B' }
>>   Bytecode:
>>     0x0000000: 2ab4 0001 b600 04ac
>>
>> at B.<init>(B.java:6)
>> at B.main(B.java:17)
>>
>> The problem is reproduced on both latest 8u and 9 (by the time of the bug
>> submission)
>>
>> My naive attempt to fix could be seen here
>> http://cr.openjdk.java.net/~apetushkov/8184989/webrev/ (based on latest 8u)
>> Please could you consider reviewing/improving it or suggesting improvement
>> direction as appropriate
>>
>> Thanks,
>> Andrey
>>
>>


More information about the compiler-dev mailing list