closes #3575.

cloneSymbol now preserves privateWithin -- need to reset it explicitly now when before it was assumed to be not to be carried over

rewrote accessibility in overriding checks so they're more readable, but
hopefully with same semantics

review by odersky

1 files changed, 31 insertions, 13 deletions

diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
index 5c577d8af4..b63f8b0add 100644
--- a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
@@ -266,12 +266,12 @@ abstract class RefChecks extends InfoTransform {
           }
         }
 
+        def accessFlagsToString(sym: Symbol)
+          = flagsToString(sym getFlag (PRIVATE | PROTECTED), if (sym.privateWithin eq NoSymbol) "" else sym.privateWithin.name.toString)
+
         def overrideAccessError() {
-          val pwString = if (other.privateWithin == NoSymbol) ""
-                         else other.privateWithin.name.toString
-          val otherAccess = flagsToString(other getFlag (PRIVATE | PROTECTED), pwString)
-          overrideError("has weaker access privileges; it should be "+
-                        (if (otherAccess == "") "public" else "at least "+otherAccess))
+          val otherAccess = accessFlagsToString(other)
+          overrideError("has weaker access privileges; it should be "+ (if (otherAccess == "") "public" else "at least "+otherAccess))
         }
 
         //Console.println(infoString(member) + " overrides " + infoString(other) + " in " + clazz);//DEBUG
@@ -302,14 +302,24 @@ abstract class RefChecks extends InfoTransform {
 
         if (typesOnly) checkOverrideTypes()
         else {
-          if (member hasFlag PRIVATE) { // (1.1)
+          // o: public | protected        | package-protected  (aka java's default access)
+          // ^-may be overridden by member with access privileges-v
+          // m: public | public/protected | public/protected/package-protected-in-same-package-as-o
+
+          if (member hasFlag PRIVATE) // (1.1)
             overrideError("has weaker access privileges; it should not be private")
-          }
-          val mb = member.accessBoundary(member.owner)
+
+          @inline def definedIn(sym: Symbol, in: Symbol) = sym != RootClass && sym != NoSymbol && sym.hasTransOwner(in)
+
           val ob = other.accessBoundary(member.owner)
-          if (mb != RootClass && mb != NoSymbol && // todo: change
-              (ob == RootClass || ob == NoSymbol || !ob.hasTransOwner(mb) ||
-               (other hasFlag PROTECTED) && !(member hasFlag PROTECTED))) {
+          val mb = member.accessBoundary(member.owner)
+          // println("checking override in "+ clazz +"\n  other: "+ infoString(other) +" ab: "+ ob.ownerChain +" flags: "+ accessFlagsToString(other))
+          // println("  overriding member: "+ infoString(member) +" ab: "+ mb.ownerChain +" flags: "+ accessFlagsToString(member))
+          // todo: align accessibility implication checking with isAccessible in Contexts
+          if (!(  mb == RootClass // m is public, definitely relaxes o's access restrictions (unless o.isJavaDefined, see below)
+               || mb == NoSymbol  // AM: what does this check?? accessBoundary does not ever seem to return NoSymbol (unless member.owner were NoSymbol)
+               || ((!(other hasFlag PROTECTED) || (member hasFlag PROTECTED)) && definedIn(ob, mb)) // (if o isProtected, so is m) and m relaxes o's access boundary
+               )) {
             overrideAccessError()
           }
           else if (other.isClass || other.isModule) {
@@ -379,7 +389,7 @@ abstract class RefChecks extends InfoTransform {
             // this overlaps somewhat with validateVariance
             if(member.isAliasType) {
               val kindErrors = typer.infer.checkKindBounds(List(member), List(memberTp.normalize), self, member.owner)
-2
+
               if(!kindErrors.isEmpty)
                 unit.error(member.pos,
                   "The kind of the right-hand side "+memberTp.normalize+" of "+member.keyString+" "+
@@ -534,11 +544,19 @@ abstract class RefChecks extends InfoTransform {
        */
       def hasMatchingSym(inclazz: Symbol, member: Symbol): Boolean =
         inclazz != clazz && {
+          lazy val memberEnclPackageCls = member.enclosingPackageClass
+
           val isVarargs = hasRepeatedParam(member.tpe)
           inclazz.info.nonPrivateDecl(member.name).filter { sym =>
-            !sym.isTerm || {
+            (!sym.isTerm || {
               val symtpe = clazz.thisType.memberType(sym)
               (member.tpe matches symtpe) || isVarargs && (toJavaRepeatedParam(member.tpe) matches symtpe)
+            }) && {
+              // http://java.sun.com/docs/books/jls/third_edition/html/names.html#6.6.5:
+              // If a public class has a [member] with default access, then this [member] is not accessible to,
+              // or inherited by a subclass declared outside this package.
+              // (sym is a java member with default access in pkg P) implies (member's enclosing package == P)
+              !(inclazz.isJavaDefined && sym.privateWithin == sym.enclosingPackageClass) || memberEnclPackageCls == sym.privateWithin
             }
           } != NoSymbol
         }