[PATCH] 8129740: Runtime exception when passing lambda in inner class constructor

[Srikanth]

I seemed to have missed this thread as I was on vacation, but have been
working on https://bugs.openjdk.java.net/browse/JDK-8129740. ATM, my belief
is that this is valid code and should be accepted and run without any 
errors.

I have captured some notes in 
https://bugs.openjdk.java.net/browse/JDK-8129740
that show the reasoning.

Thanks!
Srikanth

On Thursday 08 October 2015 02:45 PM, bsrbnd wrote:
> Any comment about this issue and the patch below?
> Thanks,
>
> bsrbnd
>
> 2015-09-17 13:46 GMT+02:00 bsrbnd <bsrbnd at gmail.com>:
>> Hi,
>>
>> Just two precisions:
>>
>> 1) If you run the compiled example, a "java.lang.VerifyError
>> (uninitializedThis)" is thrown, not a runtime exception (sorry for
>> that mistake):
>>
>> Exception in thread "main" java.lang.VerifyError: Bad type on operand stack
>> Exception Details:
>>    Location:
>>      Bug$Inner.<init>(LBug;)V @3: invokedynamic
>>    Reason:
>>      Type uninitializedThis (current frame, stack[2]) is not assignable
>> to 'Bug$Inner'
>>
>>
>> 2) The output of javap on "Inner" class gives us a good explaination.
>> The lambda expression is translated into an instance method (to access
>> instance members of the class and outer-class) and thus "this" is used
>> by "invokedynamic" before calling the constructor which leads to the
>> VerifiyError:
>>
>> public class Bug$Inner {
>>    java.lang.Object vi;
>>    final Bug this$0;
>>    public Bug$Inner(Bug, java.lang.Runnable);
>>    public Bug$Inner(Bug, java.lang.Object);
>>    public Bug$Inner(Bug);
>>    private void lambda$new$0();
>> }
>>
>>    public Bug$Inner(Bug);
>>      Code:
>>         0: aload_0
>>         1: aload_1
>>         2: aload_0
>>         3: invokedynamic #8,  0              // InvokeDynamic
>> #0:run:(LBug$Inner;)Ljava/lang/Runnable;
>>         8: invokespecial #9                  // Method
>> "<init>":(LBug;Ljava/lang/Runnable;)V
>>        11: return
>>
>> If the lambda expression doesn't access instance members of the class
>> (but static members or instance members of other classes), the lamda
>> expression is translated into a static method an the "invokedynamic"
>> doesn't need "this". For example:
>>   this(()->System.out.println(new Other().o));
>>
>> gives:
>>
>> public class Bug$Inner {
>>    java.lang.Object vi;
>>    final Bug this$0;
>>    public Bug$Inner(Bug, java.lang.Runnable);
>>    public Bug$Inner(Bug, java.lang.Object);
>>    public Bug$Inner(Bug);
>>    private static void lambda$new$0();
>> }
>>
>>    public Bug$Inner(Bug);
>>      Code:
>>         0: aload_0
>>         1: aload_1
>>         2: invokedynamic #8,  0              // InvokeDynamic
>> #0:run:()Ljava/lang/Runnable;
>>         7: invokespecial #9                  // Method
>> "<init>":(LBug;Ljava/lang/Runnable;)V
>>        10: return
>>
>> Thus, jls 8.8.7.1 seems to be respected (the error appears to be
>> "right" in issue 8129740) and then javac should (I think) give an
>> error message as shown in the patch below.
>>
>> Regards,
>> bsrbnd
>>
>> 2015-09-14 15:11 GMT+02:00 bsrbnd <bsrbnd at gmail.com>:
>>> Hi,
>>>
>>> As explained in issue 8129740, the following code produces a runtime
>>> exception ("uninitialized this"):
>>>
>>> public class Bug {
>>>      Object vb=new Object();
>>>
>>>      public class Inner {
>>>          Object vi=new Object();
>>>
>>>          public Inner(Runnable r) {r.run();}
>>>          public Inner(Object o) {System.out.println(o);}
>>>          public Inner() {
>>>              this(()->System.out.println(vb)); // KO (bug 8129740
>>> runtime exception)
>>>          }
>>>      }
>>>      public static void main(String[] args) {
>>>          new Bug().new Inner();
>>>      }
>>> }
>>>
>>> class Other {
>>>      public Object o=new Object();
>>> }
>>>
>>> With respect to JLS 8.8.7.1 (p. 264, l. 1-4), I think (if I've not
>>> missed anything) that this example should probably run without
>>> exception because "vb" is not within "Inner" class but in the
>>> outer-class.
>>>
>>> For example, all the following statements compile and run according to
>>> jls 8.8.7.1:
>>>   this(vb);
>>>   this(()->System.out.println(new Other().o));
>>>   this(new Runnable() {public void run() {System.out.println(vb);}});
>>>
>>> And the following ones doesn't compile, according to jls:
>>>   this(vi);
>>>   this(()->System.out.println(vi));
>>>   this(new Runnable() {public void run() {System.out.println(vi);}});
>>>
>>> But, if the runtime exception is "right" (as said in issue 8129740),
>>> we can detect in the attribute phase the special case of an outer
>>> member access within a lambda expression in a this/super call as shown
>>> in the following patch.
>>> Method Attr.visitLambda() keeps a track in the AttrContext if we are
>>> in a this/super call and method Attr.visitIdent() produces an error
>>> message for that case (notice that "noOuterThisPath" on line 3209
>>> never changes).
>>>
>>> Regards,
>>> bsrbnd
>>>
>>> diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
>>> b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
>>> --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
>>> +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
>>> @@ -2327,6 +2327,7 @@
>>>           }
>>>           //create an environment for attribution of the lambda expression
>>>           final Env<AttrContext> localEnv = lambdaEnv(that, env);
>>> +        localEnv.info.isLambdaInSelfCall = localEnv.info.isSelfCall;
>>>           boolean needsRecovery =
>>>                   resultInfo.checkContext.deferredAttrContext().mode ==
>>> DeferredAttr.AttrMode.CHECK;
>>>           try {
>>> @@ -3207,6 +3208,7 @@
>>>           //     (`noOuterThisPath').
>>>           Env<AttrContext> symEnv = env;
>>>           boolean noOuterThisPath = false;
>>> +        boolean isLambdaInSelfCall = symEnv.info.isLambdaInSelfCall;
>>>           if (env.enclClass.sym.owner.kind != PCK && // we are in an inner class
>>>               sym.kind.matches(KindSelector.VAL_MTH) &&
>>>               sym.owner.kind == TYP &&
>>> @@ -3238,7 +3240,7 @@
>>>           // In a constructor body,
>>>           // if symbol is a field or instance method, check that it is
>>>           // not accessed before the supertype constructor is called.
>>> -        if ((symEnv.info.isSelfCall || noOuterThisPath) &&
>>> +        if ((symEnv.info.isSelfCall || isLambdaInSelfCall ||
>>> noOuterThisPath) &&
>>>               sym.kind.matches(KindSelector.VAL_MTH) &&
>>>               sym.owner.kind == TYP &&
>>>               (sym.flags() & STATIC) == 0) {
>>> diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/AttrContext.java
>>> b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/AttrContext.java
>>> --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/AttrContext.java
>>> +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/AttrContext.java
>>> @@ -72,6 +72,8 @@
>>>        *  Is this an attribution environment for an instance creation expression?
>>>        */
>>>       boolean isNewClass = false;
>>> +
>>> +    boolean isLambdaInSelfCall = false;
>>>
>>>       /** Are arguments to current function applications boxed into an
>>> array for varargs?
>>>        */
>>> @@ -112,6 +114,7 @@
>>>           info.isSpeculative = isSpeculative;
>>>           info.isAnonymousDiamond = isAnonymousDiamond;
>>>           info.isNewClass = isNewClass;
>>> +        info.isLambdaInSelfCall = isLambdaInSelfCall;
>>>           return info;
>>>       }