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

[bsrbnd]

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;
>      }