8137269: Add better support for local caching in ArgumentAttr

ArgumentAttr should support local caches when results of speculative attribution might be thrown away

Reviewed-by: jlahoda

langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Analyzer.java

@@ -29,6 +29,7 @@ import com.sun.source.tree.LambdaExpressionTree;
 import com.sun.tools.javac.code.Source;
 import com.sun.tools.javac.code.Type;
 import com.sun.tools.javac.code.Types;
+import com.sun.tools.javac.comp.ArgumentAttr.LocalCacheContext;
 import com.sun.tools.javac.tree.JCTree;
 import com.sun.tools.javac.tree.JCTree.JCBlock;
 import com.sun.tools.javac.tree.JCTree.JCClassDecl;
@@ -54,7 +55,6 @@ import com.sun.tools.javac.tree.TreeScanner;
 import com.sun.tools.javac.util.Context;
 import com.sun.tools.javac.util.DefinedBy;
 import com.sun.tools.javac.util.DefinedBy.Api;
-import com.sun.tools.javac.util.Filter;
 import com.sun.tools.javac.util.JCDiagnostic;
 import com.sun.tools.javac.util.JCDiagnostic.DiagnosticType;
 import com.sun.tools.javac.util.List;
@@ -72,7 +72,6 @@ import static com.sun.tools.javac.code.Flags.GENERATEDCONSTR;
 import static com.sun.tools.javac.code.Flags.SYNTHETIC;
 import static com.sun.tools.javac.code.TypeTag.CLASS;
 import static com.sun.tools.javac.tree.JCTree.Tag.APPLY;
-import static com.sun.tools.javac.tree.JCTree.Tag.CLASSDEF;
 import static com.sun.tools.javac.tree.JCTree.Tag.METHODDEF;
 import static com.sun.tools.javac.tree.JCTree.Tag.NEWCLASS;
 import static com.sun.tools.javac.tree.JCTree.Tag.TYPEAPPLY;
@@ -88,6 +87,7 @@ public class Analyzer {
     final Log log;
     final Attr attr;
     final DeferredAttr deferredAttr;
+    final ArgumentAttr argumentAttr;
     final TreeMaker make;
     final Names names;
     private final boolean allowDiamondWithAnonymousClassCreation;
@@ -107,6 +107,7 @@ public class Analyzer {
         log = Log.instance(context);
         attr = Attr.instance(context);
         deferredAttr = DeferredAttr.instance(context);
+        argumentAttr = ArgumentAttr.instance(context);
         make = TreeMaker.instance(context);
         names = Names.instance(context);
         Options options = Options.instance(context);
@@ -363,8 +364,13 @@ public class Analyzer {
 
             TreeMapper treeMapper = new TreeMapper(context);
             //TODO: to further refine the analysis, try all rewriting combinations
-            deferredAttr.attribSpeculative(fakeBlock, env, attr.statInfo, treeMapper,
+            LocalCacheContext localCacheContext = argumentAttr.withLocalCacheContext();
-                    t -> new AnalyzeDeferredDiagHandler(context));
+            try {
+                deferredAttr.attribSpeculative(fakeBlock, env, attr.statInfo, treeMapper,
+                        t -> new AnalyzeDeferredDiagHandler(context));
+            } finally {
+                localCacheContext.leave();
+            }
 
             context.treeMap.entrySet().forEach(e -> {
                 context.treesToAnalyzer.get(e.getKey())

langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ArgumentAttr.java

@@ -109,9 +109,6 @@ public class ArgumentAttr extends JCTree.Visitor {
     /** Cache for argument types; behavior is influences by the currrently selected cache policy. */
     Map<UniquePos, ArgumentType<?>> argumentTypeCache = new LinkedHashMap<>();
 
-    /** Cache policy: should argument types be cached? */
-    private CachePolicy cachePolicy = CachePolicy.CACHE;
-
     public static ArgumentAttr instance(Context context) {
         ArgumentAttr instance = context.get(methodAttrKey);
         if (instance == null)
@@ -160,12 +157,29 @@ public class ArgumentAttr extends JCTree.Visitor {
     }
 
     /**
-     * Sets given ache policy and returns current policy.
+     * Returns a local caching context in which argument types can safely be cached without
+     * the risk of polluting enclosing contexts. This is useful when attempting speculative
+     * attribution of potentially erroneous expressions, which could end up polluting the cache.
      */
-    CachePolicy withCachePolicy(CachePolicy newPolicy) {
+    LocalCacheContext withLocalCacheContext() {
-        CachePolicy oldPolicy = this.cachePolicy;
+        return new LocalCacheContext();
-        this.cachePolicy = newPolicy;
+    }
-        return oldPolicy;
+
+    /**
+     * Local cache context; this class keeps track of the previous cache and reverts to it
+     * when the {@link LocalCacheContext#leave()} method is called.
+     */
+    class LocalCacheContext {
+        Map<UniquePos, ArgumentType<?>> prevCache;
+
+        public LocalCacheContext() {
+            this.prevCache = argumentTypeCache;
+            argumentTypeCache = new HashMap<>();
+        }
+
+        public void leave() {
+            argumentTypeCache = prevCache;
+        }
     }
 
     /**
@@ -226,9 +240,7 @@ public class ArgumentAttr extends JCTree.Visitor {
             setResult(that, cached.dup(that, env));
         } else {
             Z res = argumentTypeFactory.get();
-            if (cachePolicy == CachePolicy.CACHE) {
+            argumentTypeCache.put(pos, res);
-                argumentTypeCache.put(pos, res);
-            }
             setResult(that, res);
         }
     }
@@ -341,7 +353,7 @@ public class ArgumentAttr extends JCTree.Visitor {
                 speculativeTypes.put(resultInfo, t);
                 return t;
             } else {
-                if (!env.info.isSpeculative && cachePolicy == CachePolicy.CACHE) {
+                if (!env.info.isSpeculative) {
                     argumentTypeCache.remove(new UniquePos(dt.tree));
                 }
                 return deferredAttr.basicCompleter.complete(dt, resultInfo, deferredAttrContext);
@@ -663,17 +675,4 @@ public class ArgumentAttr extends JCTree.Visitor {
             return source.getFile().getName() + " @ " + source.getLineNumber(pos);
         }
     }
-
-    /**
-     * Argument type caching policy.
-     */
-    enum CachePolicy {
-        /** Cache argument types. */
-        CACHE,
-        /**
-         * Don't cache argument types. This is useful when performing speculative attribution on
-         * a tree that is known to contain erroneous info.
-         */
-        NO_CACHE;
-    }
 }

langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/DeferredAttr.java

@@ -29,7 +29,7 @@ import com.sun.source.tree.LambdaExpressionTree.BodyKind;
 import com.sun.source.tree.NewClassTree;
 import com.sun.tools.javac.code.*;
 import com.sun.tools.javac.code.Type.TypeMapping;
-import com.sun.tools.javac.comp.ArgumentAttr.CachePolicy;
+import com.sun.tools.javac.comp.ArgumentAttr.LocalCacheContext;
 import com.sun.tools.javac.comp.Resolve.ResolveError;
 import com.sun.tools.javac.resources.CompilerProperties.Fragments;
 import com.sun.tools.javac.tree.*;
@@ -777,14 +777,14 @@ public class DeferredAttr extends JCTree.Visitor {
 
             boolean canLambdaBodyCompleteNormally(JCLambda tree) {
                 List<JCVariableDecl> oldParams = tree.params;
-                CachePolicy prevPolicy = argumentAttr.withCachePolicy(CachePolicy.NO_CACHE);
+                LocalCacheContext localCacheContext = argumentAttr.withLocalCacheContext();
                 try {
                     tree.params = tree.params.stream()
                             .map(vd -> make.VarDef(vd.mods, vd.name, make.Erroneous(), null))
                             .collect(List.collector());
                     return attribSpeculativeLambda(tree, env, attr.unknownExprInfo).canCompleteNormally;
                 } finally {
-                    argumentAttr.withCachePolicy(prevPolicy);
+                    localCacheContext.leave();
                     tree.params = oldParams;
                 }
             }