Consider visibility during member resolution
Don't resolve inaccessible private or package-protected members in super
types.
MOE_MIGRATED_REVID=137218921
diff --git a/java/com/google/turbine/binder/ConstEvaluator.java b/java/com/google/turbine/binder/ConstEvaluator.java
index 274c25e..8aef355 100644
--- a/java/com/google/turbine/binder/ConstEvaluator.java
+++ b/java/com/google/turbine/binder/ConstEvaluator.java
@@ -56,10 +56,10 @@
public class ConstEvaluator {
/** The symbol of the enclosing class. */
- private final ClassSymbol sym;
+ private final ClassSymbol owner;
/** The bound node of the enclosing class. */
- private final SourceTypeBoundClass owner;
+ private final SourceTypeBoundClass base;
/** The constant variable environment. */
private final Env<FieldSymbol, Const.Value> values;
@@ -68,13 +68,13 @@
private final CompoundEnv<ClassSymbol, TypeBoundClass> env;
public ConstEvaluator(
- ClassSymbol sym,
- SourceTypeBoundClass owner,
+ ClassSymbol owner,
+ SourceTypeBoundClass base,
Env<FieldSymbol, Const.Value> values,
CompoundEnv<ClassSymbol, TypeBoundClass> env) {
- this.sym = sym;
this.owner = owner;
+ this.base = base;
this.values = values;
this.env = env;
}
@@ -147,7 +147,7 @@
{
ClassTy classTy = (ClassTy) t.type();
ClassSymbol classSym =
- HierarchyBinder.resolveClass(owner.source(), env, owner.scope(), sym, classTy);
+ HierarchyBinder.resolveClass(base.source(), env, base.scope(), owner, classTy);
return new Const.ClassValue(Type.ClassTy.asNonParametricClassTy(classSym));
}
default:
@@ -172,34 +172,34 @@
FieldInfo resolveField(ConstVarName t) {
String simpleName = t.name().get(0);
- FieldInfo field = lexicalField(env, sym, simpleName);
+ FieldInfo field = lexicalField(env, owner, simpleName);
if (field != null) {
return field;
}
- LookupResult result = owner.scope().lookup(new LookupKey(t.name()));
+ LookupResult result = base.scope().lookup(new LookupKey(t.name()));
if (result != null) {
ClassSymbol sym = (ClassSymbol) result.sym();
for (int i = 0; i < result.remaining().size() - 1; i++) {
- sym = Resolve.resolve(env, sym, result.remaining().get(i));
+ sym = Resolve.resolve(env, sym, sym, result.remaining().get(i));
if (sym == null) {
return null;
}
}
- field = Resolve.resolveField(env, sym, Iterables.getLast(result.remaining()));
+ field = Resolve.resolveField(env, owner, sym, Iterables.getLast(result.remaining()));
if (field != null) {
return field;
}
}
- ClassSymbol classSymbol = owner.memberImports().singleMemberImport(simpleName);
+ ClassSymbol classSymbol = base.memberImports().singleMemberImport(simpleName);
if (classSymbol != null) {
- field = Resolve.resolveField(env, classSymbol, simpleName);
+ field = Resolve.resolveField(env, owner, classSymbol, simpleName);
if (field != null) {
return field;
}
}
- Iterator<ClassSymbol> it = owner.memberImports().onDemandImports();
+ Iterator<ClassSymbol> it = base.memberImports().onDemandImports();
while (it.hasNext()) {
- field = Resolve.resolveField(env, it.next(), simpleName);
+ field = Resolve.resolveField(env, owner, it.next(), simpleName);
if (field != null) {
return field;
}
@@ -208,11 +208,11 @@
}
/** Search for constant variables in lexically enclosing scopes. */
- private static FieldInfo lexicalField(
+ private FieldInfo lexicalField(
Env<ClassSymbol, TypeBoundClass> env, ClassSymbol sym, String name) {
while (sym != null) {
TypeBoundClass info = env.get(sym);
- FieldInfo field = Resolve.resolveField(env, sym, name);
+ FieldInfo field = Resolve.resolveField(env, owner, sym, name);
if (field != null) {
return field;
}
@@ -844,10 +844,10 @@
}
private Const.AnnotationValue evalAnno(Tree.Anno t) {
- LookupResult result = owner.scope().lookup(new LookupKey(t.name()));
+ LookupResult result = base.scope().lookup(new LookupKey(t.name()));
ClassSymbol sym = (ClassSymbol) result.sym();
for (String name : result.remaining()) {
- sym = Resolve.resolve(env, sym, name);
+ sym = Resolve.resolve(env, sym, sym, name);
}
AnnoInfo annoInfo = evaluateAnnotation(sym, t.args());
return new Const.AnnotationValue(annoInfo.sym(), annoInfo.values());
@@ -890,6 +890,6 @@
}
private TurbineError error(int position, String message, Object... args) {
- return TurbineError.format(owner.source(), position, message, args);
+ return TurbineError.format(base.source(), position, message, args);
}
}
diff --git a/java/com/google/turbine/binder/HierarchyBinder.java b/java/com/google/turbine/binder/HierarchyBinder.java
index 386b051..f3e44d4 100644
--- a/java/com/google/turbine/binder/HierarchyBinder.java
+++ b/java/com/google/turbine/binder/HierarchyBinder.java
@@ -228,7 +228,7 @@
// This needs to consider member type declarations inherited from supertypes and interfaces.
ClassSymbol sym = (ClassSymbol) result.sym();
for (String bit : result.remaining()) {
- sym = Resolve.resolve(env, sym, bit);
+ sym = Resolve.resolve(env, owner, sym, bit);
if (sym == null) {
throw TurbineError.format(
source, ty.position(), String.format("symbol not found %s\n", bit));
@@ -241,13 +241,13 @@
public static LookupResult lookup(
Env<ClassSymbol, ? extends HeaderBoundClass> env,
CompoundScope parent,
- ClassSymbol sym,
+ ClassSymbol owner,
LookupKey lookup) {
// Handle any lexically enclosing class declarations (if we're binding a member class).
// We could build out scopes for this, but it doesn't seem worth it. (And sharing the scopes
// with other members of the same enclosing declaration would be complicated.)
- for (ClassSymbol curr = sym; curr != null; curr = env.get(curr).owner()) {
- ClassSymbol result = Resolve.resolve(env, curr, lookup.first());
+ for (ClassSymbol curr = owner; curr != null; curr = env.get(curr).owner()) {
+ ClassSymbol result = Resolve.resolve(env, owner, curr, lookup.first());
if (result != null) {
return new LookupResult(result, lookup);
}
diff --git a/java/com/google/turbine/binder/Resolve.java b/java/com/google/turbine/binder/Resolve.java
index 5f8ad31..f54f1cd 100644
--- a/java/com/google/turbine/binder/Resolve.java
+++ b/java/com/google/turbine/binder/Resolve.java
@@ -25,6 +25,8 @@
import com.google.turbine.binder.lookup.CanonicalSymbolResolver;
import com.google.turbine.binder.lookup.LookupResult;
import com.google.turbine.binder.sym.ClassSymbol;
+import com.google.turbine.model.TurbineVisibility;
+import java.util.Objects;
/** Qualified name resolution. */
public class Resolve {
@@ -35,7 +37,10 @@
* superclasses or interfaces.
*/
public static ClassSymbol resolve(
- Env<ClassSymbol, ? extends HeaderBoundClass> env, ClassSymbol sym, String simpleName) {
+ Env<ClassSymbol, ? extends HeaderBoundClass> env,
+ ClassSymbol origin,
+ ClassSymbol sym,
+ String simpleName) {
ClassSymbol result;
HeaderBoundClass bound = env.get(sym);
if (bound == null) {
@@ -46,14 +51,14 @@
return result;
}
if (bound.superclass() != null) {
- result = resolve(env, bound.superclass(), simpleName);
- if (result != null) {
+ result = resolve(env, origin, bound.superclass(), simpleName);
+ if (result != null && visible(origin, result, env.get(result))) {
return result;
}
}
for (ClassSymbol i : bound.interfaces()) {
- result = resolve(env, i, simpleName);
- if (result != null) {
+ result = resolve(env, origin, i, simpleName);
+ if (result != null && visible(origin, result, env.get(result))) {
return result;
}
}
@@ -99,7 +104,7 @@
* superclasses or interfaces.
*/
public static FieldInfo resolveField(
- Env<ClassSymbol, TypeBoundClass> env, ClassSymbol sym, String name) {
+ Env<ClassSymbol, TypeBoundClass> env, ClassSymbol origin, ClassSymbol sym, String name) {
TypeBoundClass info = env.get(sym);
for (FieldInfo f : info.fields()) {
if (f.name().equals(name)) {
@@ -107,17 +112,55 @@
}
}
if (info.superclass() != null) {
- FieldInfo field = resolveField(env, info.superclass(), name);
- if (field != null) {
+ FieldInfo field = resolveField(env, origin, info.superclass(), name);
+ if (field != null && visible(origin, field)) {
return field;
}
}
for (ClassSymbol i : info.interfaces()) {
- FieldInfo field = resolveField(env, i, name);
- if (field != null) {
+ FieldInfo field = resolveField(env, origin, i, name);
+ if (field != null && visible(origin, field)) {
return field;
}
}
return null;
}
+
+ /** Is the given field visible when inherited into class origin? */
+ private static boolean visible(ClassSymbol origin, FieldInfo info) {
+ return visible(origin, info.sym().owner(), info.access());
+ }
+
+ /** Is the given type visible when inherited into class origin? */
+ private static boolean visible(ClassSymbol origin, ClassSymbol sym, HeaderBoundClass info) {
+ return visible(origin, sym, info.access());
+ }
+
+ private static boolean visible(ClassSymbol origin, ClassSymbol owner, int access) {
+ TurbineVisibility visibility = TurbineVisibility.fromAccess(access);
+ switch (visibility) {
+ case PUBLIC:
+ case PROTECTED:
+ return true;
+ case PACKAGE:
+ return Objects.equals(packageName(owner), packageName(origin));
+ case PRIVATE:
+ // Private members of lexically enclosing declarations are not handled,
+ // since this visibility check is only used for inherited members.
+ return owner.equals(origin);
+ default:
+ throw new AssertionError(visibility);
+ }
+ }
+
+ private static String packageName(ClassSymbol sym) {
+ if (sym == null) {
+ return null;
+ }
+ int idx = sym.binaryName().lastIndexOf('/');
+ if (idx == -1) {
+ return null;
+ }
+ return sym.binaryName().substring(0, idx);
+ }
}
diff --git a/java/com/google/turbine/binder/TypeBinder.java b/java/com/google/turbine/binder/TypeBinder.java
index 545a488..b55fbcc 100644
--- a/java/com/google/turbine/binder/TypeBinder.java
+++ b/java/com/google/turbine/binder/TypeBinder.java
@@ -113,7 +113,7 @@
if (result != null) {
return new LookupResult(result, lookup);
}
- result = Resolve.resolve(env, curr, lookup.first());
+ result = Resolve.resolve(env, sym, curr, lookup.first());
if (result != null) {
return new LookupResult(result, lookup);
}
@@ -180,7 +180,8 @@
new Scope() {
@Override
public LookupResult lookup(LookupKey lookup) {
- ClassSymbol result = Resolve.resolve(env, base.superclass(), lookup.first());
+ ClassSymbol result =
+ Resolve.resolve(env, owner, base.superclass(), lookup.first());
if (result != null) {
return new LookupResult(result, lookup);
}
@@ -497,7 +498,7 @@
LookupResult lookupResult = scope.lookup(new LookupKey(tree.name()));
ClassSymbol sym = (ClassSymbol) lookupResult.sym();
for (String name : lookupResult.remaining()) {
- sym = Resolve.resolve(env, sym, name);
+ sym = Resolve.resolve(env, owner, sym, name);
}
result.add(new TypeBoundClass.AnnoInfo(sym, tree.args(), null));
}
@@ -581,7 +582,7 @@
classes.add(new Type.ClassTy.SimpleClassTy(sym, bindTyArgs(scope, flat.get(idx++).tyargs())));
for (; idx < flat.size(); idx++) {
Tree.ClassTy curr = flat.get(idx);
- sym = Resolve.resolve(env, sym, curr.name());
+ sym = Resolve.resolve(env, owner, sym, curr.name());
classes.add(new Type.ClassTy.SimpleClassTy(sym, bindTyArgs(scope, curr.tyargs())));
}
return new Type.ClassTy(classes.build());
diff --git a/java/com/google/turbine/binder/lookup/LookupKey.java b/java/com/google/turbine/binder/lookup/LookupKey.java
index 0646ece..94e0838 100644
--- a/java/com/google/turbine/binder/lookup/LookupKey.java
+++ b/java/com/google/turbine/binder/lookup/LookupKey.java
@@ -26,7 +26,7 @@
*/
@Immutable
public class LookupKey {
- final ImmutableList<String> simpleNames;
+ private final ImmutableList<String> simpleNames;
public LookupKey(Iterable<String> simpleNames) {
this.simpleNames = ImmutableList.copyOf(simpleNames);
@@ -49,9 +49,9 @@
* remaining nested type names (which may not be canonical).
*
* <ul>
- * <li>{@code ((PACKAGE java) (KEY util.HashMap.Entry))}
- * <li>{@code ((PACKAGE java.util) (KEY HashMap.Entry))}
- * <li>{@code ((CLASS java.util.HashMap) (KEY Entry))}
+ * <li>{@code ((PACKAGE java) (KEY util.HashMap.Entry))}
+ * <li>{@code ((PACKAGE java.util) (KEY HashMap.Entry))}
+ * <li>{@code ((CLASS java.util.HashMap) (KEY Entry))}
* </ul>
*/
public LookupKey rest() {
diff --git a/javatests/com/google/turbine/lower/LowerIntegrationTest.java b/javatests/com/google/turbine/lower/LowerIntegrationTest.java
index fd1acb8..d48a51f 100644
--- a/javatests/com/google/turbine/lower/LowerIntegrationTest.java
+++ b/javatests/com/google/turbine/lower/LowerIntegrationTest.java
@@ -234,6 +234,12 @@
// http://forge.ow2.org/tracker/?func=detail&aid=317776&group_id=23&atid=100023
// "canon_array.test",
"java_lang_object.test",
+ "visible_package.test",
+ "visible_private.test",
+ "visible_same_package.test",
+ "private_member.test",
+ "visible_nested.test",
+ "visible_qualified.test",
};
List<Object[]> tests =
ImmutableList.copyOf(testCases).stream().map(x -> new Object[] {x}).collect(toList());
diff --git a/javatests/com/google/turbine/lower/testdata/private_member.test b/javatests/com/google/turbine/lower/testdata/private_member.test
new file mode 100644
index 0000000..cad64f0
--- /dev/null
+++ b/javatests/com/google/turbine/lower/testdata/private_member.test
@@ -0,0 +1,10 @@
+=== Test.java ===
+class Test {
+ private static class I {}
+ private static final int CONST = 42;
+
+ public Test(I i) {}
+ public void f(I i) {}
+ public static final int X = CONST;
+}
+
diff --git a/javatests/com/google/turbine/lower/testdata/visible_nested.test b/javatests/com/google/turbine/lower/testdata/visible_nested.test
new file mode 100644
index 0000000..945a7dd
--- /dev/null
+++ b/javatests/com/google/turbine/lower/testdata/visible_nested.test
@@ -0,0 +1,13 @@
+=== Test.java ===
+public class Test {
+ private class Private {}
+ private static final int CONST = 42;
+ private class Inner {
+ private class InnerMost {
+ // access private member of enclosing instance
+ Private p;
+ static final int X = CONST;
+ }
+ }
+}
+
diff --git a/javatests/com/google/turbine/lower/testdata/visible_package.test b/javatests/com/google/turbine/lower/testdata/visible_package.test
new file mode 100644
index 0000000..61b4b9a
--- /dev/null
+++ b/javatests/com/google/turbine/lower/testdata/visible_package.test
@@ -0,0 +1,22 @@
+=== a/A.java ===
+package a;
+public class A {
+ public static final int CONST = 2;
+}
+
+=== b/B.java ===
+package b;
+public class B {
+ static final int CONST = 1;
+ static class A {}
+}
+
+=== Package.java ===
+package p;
+import a.A;
+import b.B;
+import static a.A.CONST;
+public class Package extends B {
+ void f(A i) {}
+ final int x = CONST;
+}
diff --git a/javatests/com/google/turbine/lower/testdata/visible_private.test b/javatests/com/google/turbine/lower/testdata/visible_private.test
new file mode 100644
index 0000000..2bb96e1
--- /dev/null
+++ b/javatests/com/google/turbine/lower/testdata/visible_private.test
@@ -0,0 +1,21 @@
+=== a/A.java ===
+package a;
+public class A {
+ public static final int CONST = 2;
+}
+
+=== c/C.java ===
+package c;
+public class C {
+ private static class A {}
+ private static final int CONST = 1;
+}
+
+=== Private.java ===
+package p;
+import a.A;
+import c.C;
+public class Private extends C {
+ void f(A i) {}
+}
+
diff --git a/javatests/com/google/turbine/lower/testdata/visible_qualified.test b/javatests/com/google/turbine/lower/testdata/visible_qualified.test
new file mode 100644
index 0000000..799143a
--- /dev/null
+++ b/javatests/com/google/turbine/lower/testdata/visible_qualified.test
@@ -0,0 +1,8 @@
+=== Test.java ===
+public class Test {
+ public static class Super {
+ private static final double PI = 3.14159;
+ }
+
+ public static final double A = Super.PI;
+}
diff --git a/javatests/com/google/turbine/lower/testdata/visible_same_package.test b/javatests/com/google/turbine/lower/testdata/visible_same_package.test
new file mode 100644
index 0000000..9872b1b
--- /dev/null
+++ b/javatests/com/google/turbine/lower/testdata/visible_same_package.test
@@ -0,0 +1,22 @@
+=== a/A.java ===
+package a;
+public class A {
+ public static final int CONST = 2;
+}
+
+=== P/P.java ===
+package p;
+public class P {
+ static class A {}
+ static final int CONST = 1;
+}
+
+=== SamePackage.java ===
+package p;
+import a.A;
+import static a.A.CONST;
+public class SamePackage extends P {
+ void f(A i) {}
+ final int x = CONST;
+}
+