8004962: Code generation crash with lambda and local classes

Summary: Translation info should be propagated from LambdaToMethod to Lower
Reviewed-by: jjg, rfield

src/share/classes/com/sun/tools/javac/comp/LambdaToMethod.java

@@ -44,6 +44,7 @@ import com.sun.tools.javac.code.Type.ClassType;
 import com.sun.tools.javac.code.Type.MethodType;
 import com.sun.tools.javac.code.Types;
 import com.sun.tools.javac.comp.LambdaToMethod.LambdaAnalyzer.*;
+import com.sun.tools.javac.comp.Lower.BasicFreeVarCollector;
 import com.sun.tools.javac.jvm.*;
 import com.sun.tools.javac.util.*;
 import com.sun.tools.javac.util.List;
@@ -70,6 +71,7 @@ import static com.sun.tools.javac.tree.JCTree.Tag.*;
  */
 public class LambdaToMethod extends TreeTranslator {
 
+    private Lower lower;
     private Names names;
     private Symtab syms;
     private Resolve rs;
@@ -147,6 +149,7 @@ public class LambdaToMethod extends TreeTranslator {
     }
 
     private LambdaToMethod(Context context) {
+        lower = Lower.instance(context);
         names = Names.instance(context);
         syms = Symtab.instance(context);
         rs = Resolve.instance(context);
@@ -1037,6 +1040,8 @@ public class LambdaToMethod extends TreeTranslator {
         private Map<String, Integer> serializableLambdaCounts =
                 new HashMap<String, Integer>();
 
+        private Map<Symbol, JCClassDecl> localClassDefs;
+
         /**
          * maps for fake clinit symbols to be used as owners of lambda occurring in
          * a static var init context
@@ -1046,6 +1051,7 @@ public class LambdaToMethod extends TreeTranslator {
 
         private void analyzeClass(JCClassDecl tree) {
             frameStack = List.nil();
+            localClassDefs = new HashMap<Symbol, JCClassDecl>();
             scan(tree);
         }
 
@@ -1072,13 +1078,22 @@ public class LambdaToMethod extends TreeTranslator {
             try {
                 serializableLambdaCounts = new HashMap<String, Integer>();
                 prevClinits = new HashMap<ClassSymbol, Symbol>();
+                if (tree.sym.owner.kind == MTH) {
+                    localClassDefs.put(tree.sym, tree);
+                }
                 if (directlyEnclosingLambda() != null) {
                     tree.sym.owner = owner();
                     if (tree.sym.hasOuterInstance()) {
                         //if a class is defined within a lambda, the lambda must capture
                         //its enclosing instance (if any)
-                        ((LambdaTranslationContext) context())
-                                .addSymbol(tree.sym.type.getEnclosingType().tsym, CAPTURED_THIS);
+                        TranslationContext<?> localContext = context();
+                        while (localContext != null) {
+                            if (localContext.tree.getTag() == LAMBDA) {
+                                ((LambdaTranslationContext)localContext)
+                                        .addSymbol(tree.sym.type.getEnclosingType().tsym, CAPTURED_THIS);
+                            }
+                            localContext = localContext.prev;
+                        }
                     }
                 }
                 frameStack = frameStack.prepend(new Frame(tree));
@@ -1164,11 +1179,50 @@ public class LambdaToMethod extends TreeTranslator {
         @Override
         public void visitNewClass(JCNewClass tree) {
             if (lambdaNewClassFilter(context(), tree)) {
-                ((LambdaTranslationContext) context())
-                        .addSymbol(tree.type.getEnclosingType().tsym, CAPTURED_THIS);
+                TranslationContext<?> localContext = context();
+                while (localContext != null) {
+                    if (localContext.tree.getTag() == LAMBDA) {
+                        ((LambdaTranslationContext)localContext)
+                                .addSymbol(tree.type.getEnclosingType().tsym, CAPTURED_THIS);
+                    }
+                    localContext = localContext.prev;
+                }
+            }
+            if (context() != null && tree.type.tsym.owner.kind == MTH) {
+                LambdaTranslationContext lambdaContext = (LambdaTranslationContext)context();
+                captureLocalClassDefs(tree.type.tsym, lambdaContext);
             }
             super.visitNewClass(tree);
         }
+        //where
+            void captureLocalClassDefs(Symbol csym, final LambdaTranslationContext lambdaContext) {
+                JCClassDecl localCDef = localClassDefs.get(csym);
+                if (localCDef != null && localCDef.pos < lambdaContext.tree.pos) {
+                    BasicFreeVarCollector fvc = lower.new BasicFreeVarCollector() {
+                        @Override
+                        void addFreeVars(ClassSymbol c) {
+                            captureLocalClassDefs(c, lambdaContext);
+                        }
+                        @Override
+                        void visitSymbol(Symbol sym) {
+                            if (sym.kind == VAR &&
+                                    sym.owner.kind == MTH &&
+                                    ((VarSymbol)sym).getConstValue() == null) {
+                                TranslationContext<?> localContext = context();
+                                while (localContext != null) {
+                                    if (localContext.tree.getTag() == LAMBDA) {
+                                        JCTree block = capturedDecl(localContext.depth, sym);
+                                        if (block == null) break;
+                                        ((LambdaTranslationContext)localContext).addSymbol(sym, CAPTURED_VAR);
+                                    }
+                                    localContext = localContext.prev;
+                                }
+                            }
+                        }
+                    };
+                    fvc.scan(localCDef);
+                }
+            }
 
         @Override
         public void visitReference(JCMemberReference tree) {
@@ -1550,7 +1604,7 @@ public class LambdaToMethod extends TreeTranslator {
              * Translate a symbol of a given kind into something suitable for the
              * synthetic lambda body
              */
-            Symbol translate(String name, Symbol sym, LambdaSymbolKind skind) {
+            Symbol translate(String name, final Symbol sym, LambdaSymbolKind skind) {
                 switch (skind) {
                     case CAPTURED_THIS:
                         return sym;  // self represented
@@ -1558,6 +1612,14 @@ public class LambdaToMethod extends TreeTranslator {
                         // Just erase the type var
                         return new VarSymbol(sym.flags(), names.fromString(name),
                                 types.erasure(sym.type), sym.owner);
+                    case CAPTURED_VAR:
+                        return new VarSymbol(SYNTHETIC | FINAL, names.fromString(name), types.erasure(sym.type), translatedSym) {
+                            @Override
+                            public Symbol baseSymbol() {
+                                //keep mapping with original captured symbol
+                                return sym;
+                            }
+                        };
                     default:
                         return makeSyntheticVar(FINAL, name, types.erasure(sym.type), translatedSym);
                 }

src/share/classes/com/sun/tools/javac/comp/Lower.java

@@ -163,6 +163,12 @@ public class Lower extends TreeTranslator {
      */
     JCTree outermostMemberDef;
 
+    /** A map from local variable symbols to their translation (as per LambdaToMethod).
+     * This is required when a capturing local class is created from a lambda (in which
+     * case the captured symbols should be replaced with the translated lambda symbols).
+     */
+    Map<Symbol, Symbol> lambdaTranslationMap = null;
+
     /** A navigator class for assembling a mapping from local class symbols
      *  to class definition trees.
      *  There is only one case; all other cases simply traverse down the tree.
@@ -206,10 +212,51 @@ public class Lower extends TreeTranslator {
     Map<ClassSymbol,List<VarSymbol>> freevarCache;
 
     /** A navigator class for collecting the free variables accessed
-     *  from a local class.
-     *  There is only one case; all other cases simply traverse down the tree.
+     *  from a local class. There is only one case; all other cases simply
+     *  traverse down the tree. This class doesn't deal with the specific
+     *  of Lower - it's an abstract visitor that is meant to be reused in
+     *  order to share the local variable capture logic.
+     */
+    abstract class BasicFreeVarCollector extends TreeScanner {
+
+        /** Add all free variables of class c to fvs list
+         *  unless they are already there.
+         */
+        abstract void addFreeVars(ClassSymbol c);
+
+        /** If tree refers to a variable in owner of local class, add it to
+         *  free variables list.
+         */
+        public void visitIdent(JCIdent tree) {
+            visitSymbol(tree.sym);
+        }
+        // where
+        abstract void visitSymbol(Symbol _sym);
+
+        /** If tree refers to a class instance creation expression
+         *  add all free variables of the freshly created class.
+         */
+        public void visitNewClass(JCNewClass tree) {
+            ClassSymbol c = (ClassSymbol)tree.constructor.owner;
+            addFreeVars(c);
+            super.visitNewClass(tree);
+        }
+
+        /** If tree refers to a superclass constructor call,
+         *  add all free variables of the superclass.
+         */
+        public void visitApply(JCMethodInvocation tree) {
+            if (TreeInfo.name(tree.meth) == names._super) {
+                addFreeVars((ClassSymbol) TreeInfo.symbol(tree.meth).owner);
+            }
+            super.visitApply(tree);
+        }
+    }
+
+    /**
+     * Lower-specific subclass of {@code BasicFreeVarCollector}.
      */
-    class FreeVarCollector extends TreeScanner {
+    class FreeVarCollector extends BasicFreeVarCollector {
 
         /** The owner of the local class.
          */
@@ -238,10 +285,8 @@ public class Lower extends TreeTranslator {
             fvs = fvs.prepend(v);
         }
 
-        /** Add all free variables of class c to fvs list
-         *  unless they are already there.
-         */
-        private void addFreeVars(ClassSymbol c) {
+        @Override
+        void addFreeVars(ClassSymbol c) {
             List<VarSymbol> fvs = freevarCache.get(c);
             if (fvs != null) {
                 for (List<VarSymbol> l = fvs; l.nonEmpty(); l = l.tail) {
@@ -250,15 +295,8 @@ public class Lower extends TreeTranslator {
             }
         }
 
-        /** If tree refers to a variable in owner of local class, add it to
-         *  free variables list.
-         */
-        public void visitIdent(JCIdent tree) {
-            result = tree;
-            visitSymbol(tree.sym);
-        }
-        // where
-        private void visitSymbol(Symbol _sym) {
+        @Override
+        void visitSymbol(Symbol _sym) {
             Symbol sym = _sym;
             if (sym.kind == VAR || sym.kind == MTH) {
                 while (sym != null && sym.owner != owner)
@@ -281,7 +319,6 @@ public class Lower extends TreeTranslator {
          */
         public void visitNewClass(JCNewClass tree) {
             ClassSymbol c = (ClassSymbol)tree.constructor.owner;
-            addFreeVars(c);
             if (tree.encl == null &&
                 c.hasOuterInstance() &&
                 outerThisStack.head != null)
@@ -306,7 +343,6 @@ public class Lower extends TreeTranslator {
          */
         public void visitApply(JCMethodInvocation tree) {
             if (TreeInfo.name(tree.meth) == names._super) {
-                addFreeVars((ClassSymbol) TreeInfo.symbol(tree.meth).owner);
                 Symbol constructor = TreeInfo.symbol(tree.meth);
                 ClassSymbol c = (ClassSymbol)constructor.owner;
                 if (c.hasOuterInstance() &&
@@ -1171,6 +1207,14 @@ public class Lower extends TreeTranslator {
                             accessBase(tree.pos(), sym), sym).setType(tree.type);
                     }
                 }
+            } else if (sym.owner.kind == MTH && lambdaTranslationMap != null) {
+                //sym is a local variable - check the lambda translation map to
+                //see if sym has been translated to something else in the current
+                //scope (by LambdaToMethod)
+                Symbol translatedSym = lambdaTranslationMap.get(sym);
+                if (translatedSym != null) {
+                    tree = make.at(tree.pos).Ident(translatedSym);
+                }
             }
         }
         return tree;
@@ -2725,10 +2769,30 @@ public class Lower extends TreeTranslator {
 
             outerThisStack = prevOuterThisStack;
         } else {
-            super.visitMethodDef(tree);
+            Map<Symbol, Symbol> prevLambdaTranslationMap =
+                    lambdaTranslationMap;
+            try {
+                lambdaTranslationMap = (tree.sym.flags() & SYNTHETIC) != 0 &&
+                        tree.sym.name.startsWith(names.lambda) ?
+                        makeTranslationMap(tree) : null;
+                super.visitMethodDef(tree);
+            } finally {
+                lambdaTranslationMap = prevLambdaTranslationMap;
+            }
         }
         result = tree;
     }
+    //where
+        private Map<Symbol, Symbol> makeTranslationMap(JCMethodDecl tree) {
+            Map<Symbol, Symbol> translationMap = new HashMap<Symbol,Symbol>();
+            for (JCVariableDecl vd : tree.params) {
+                Symbol p = vd.sym;
+                if (p != p.baseSymbol()) {
+                    translationMap.put(p.baseSymbol(), p);
+                }
+            }
+            return translationMap;
+        }
 
     public void visitAnnotatedType(JCAnnotatedType tree) {
         // No need to retain type annotations any longer.

test/tools/javac/lambda/LambdaCapture07.java

+/*
+ * Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8004962
+ * @summary Code generation crash with lambda and local classes
+ */
+public class LambdaCapture07 {
+
+    static int assertionCount = 0;
+
+    static void assertTrue(boolean cond) {
+        assertionCount++;
+        if (!cond)
+            throw new AssertionError();
+    }
+
+    interface SAM {
+        void m();
+    }
+
+    void test(int i) {
+        class Local { Local() { assertTrue(i == 42); } }
+        class LocalSub extends Local { }
+        SAM s_sup = ()->new Local();
+        s_sup.m();
+        SAM s_sub = ()->new LocalSub();
+        s_sub.m();
+        SAM s_sup_nested = ()->{ SAM s = ()->new Local(); s.m(); };
+        s_sup_nested.m();
+        SAM s_sub_nested = ()->{ SAM s = ()->new LocalSub(); s.m(); };
+        s_sub_nested.m();
+    }
+
+    public static void main(String[] args) {
+        new LambdaCapture07().test(42);
+        assertTrue(assertionCount == 4);
+    }
+}