RFR: 8263614: javac allows local variables to be accessed from a static context [v2]

[Vicente Romero]

On Thu, 13 May 2021 20:08:56 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java line 2329:
>> 
>>> 2327:                     (sym.owner.kind == MTH &&
>>> 2328:                     currentEnv != originalEnv &&
>>> 2329:                         (!isInnerClassOfMethod(sym.owner, originalEnv.tree.hasTag(CLASSDEF) ?
>> 
>> Can't you do a similar approach here and use staticOnly to rule out tvars?
>
> I think you can go in findType, move the assignment to staticOnly to *after* the call to findTypeVar - then you can drop all type var symbol you find with staticOnly = true.

I think that type variables are trickier than local variables and probably won't be as easy to tame. I tried several combinations of the patch below which I think is what you have in mind:


diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java
index 70ea875a4b3..832a8ba8ede 100644
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java
@@ -2316,29 +2316,6 @@ public class Resolve {
         return bestSoFar;
     }
 
-    Symbol findTypeVar(Env<AttrContext> currentEnv, Env<AttrContext> originalEnv, Name name, boolean staticOnly) {
-        for (Symbol sym : currentEnv.info.scope.getSymbolsByName(name)) {
-            if (sym.kind == TYP) {
-                if (staticOnly &&
-                    sym.type.hasTag(TYPEVAR) &&
-                    ((sym.owner.kind == TYP) ||
-                    // are we trying to access a TypeVar defined in a method from a local static type: interface, enum or record?
-                    allowRecords &&
-                    (sym.owner.kind == MTH &&
-                    currentEnv != originalEnv &&
-                        (!isInnerClassOfMethod(sym.owner, originalEnv.tree.hasTag(CLASSDEF) ?
-                        ((JCClassDecl)originalEnv.tree).sym :
-                        originalEnv.enclClass.sym) ||
-                        originalEnv.info.staticLevel > currentEnv.info.staticLevel)
-                    ))) {
-                    return new StaticError(sym);
-                }
-                return sym;
-            }
-        }
-        return typeNotFound;
-    }
-
     boolean isInnerClassOfMethod(Symbol msym, Symbol csym) {
         while (csym.owner != msym) {
             if (csym.isStatic()) return false;
@@ -2358,18 +2335,25 @@ public class Resolve {
         Symbol sym;
         boolean staticOnly = false;
         for (Env<AttrContext> env1 = env; env1.outer != null; env1 = env1.outer) {
-            if (isStatic(env1)) staticOnly = true;
             // First, look for a type variable and the first member type
-            final Symbol tyvar = findTypeVar(env1, env, name, staticOnly);
+            Symbol tyvar = typeNotFound;
+            for (Symbol s : env1.info.scope.getSymbolsByName(name)) {
+                if (s.kind == TYP) {
+                    tyvar = s;
+                }
+            }
             sym = findImmediateMemberType(env1, env1.enclClass.sym.type,
                                           name, env1.enclClass.sym);
-
+            if (isStatic(env1)) staticOnly = true;
             // Return the type variable if we have it, and have no
             // immediate member, OR the type variable is for a method.
             if (tyvar != typeNotFound) {
                 if (env.baseClause || sym == typeNotFound ||
                     (tyvar.kind == TYP && tyvar.exists() &&
                      tyvar.owner.kind == MTH)) {
+                    if (staticOnly) {
+                        return new StaticError(tyvar);
+                    }
                     return tyvar;
                 }
             }


if the line: `if (isStatic(env1)) staticOnly = true;` is left in the position it appears in the patch above then valid code like:


    public static <A> List<A> nil() {
        return (List<A>)EMPTY_LIST;
    }

will stop being accepted as the type variable is being accessed from a static context, but this case is kosher. On the other hand if line `if (isStatic(env1)) staticOnly = true;` if moved after the if block:


if (tyvar != typeNotFound) {


then the compiler will accept the code below when it should fail:


import java.util.Collection;
class Test<T extends Collection> {
    static void test(T t) { t.iterator(); }
}

-------------

PR: https://git.openjdk.java.net/jdk/pull/4004