Skip to content

Fix #997 #1066

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Feb 19, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions src/dotty/tools/dotc/ast/Desugar.scala
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,7 @@ object desugar {
def classDef(cdef: TypeDef)(implicit ctx: Context): Tree = {
val TypeDef(name, impl @ Template(constr0, parents, self, _)) = cdef
val mods = cdef.mods
val accessFlags = (mods.flags & AccessFlags).toCommonFlags

val (constr1, defaultGetters) = defDef(constr0, isPrimaryConstructor = true) match {
case meth: DefDef => (meth, Nil)
Expand Down Expand Up @@ -312,6 +313,7 @@ object desugar {
case ValDef(_, tpt, _) => isRepeated(tpt)
case _ => false
})

val copyMeths =
if (mods.is(Abstract) || hasRepeatedParam) Nil // cannot have default arguments for repeated parameters, hence copy method is not issued
else {
Expand Down Expand Up @@ -346,7 +348,7 @@ object desugar {
moduleDef(
ModuleDef(
name.toTermName, Template(emptyConstructor, parentTpt :: Nil, EmptyValDef, defs))
.withMods(synthetic))
.withFlags(Synthetic | accessFlags))
.withPos(cdef.pos).toList

// The companion object definitions, if a companion is needed, Nil otherwise.
Expand All @@ -371,7 +373,7 @@ object desugar {
if (mods is Abstract) Nil
else
DefDef(nme.apply, derivedTparams, derivedVparamss, TypeTree(), creatorExpr)
.withMods(synthetic | (constr1.mods.flags & DefaultParameterized)) :: Nil
.withFlags(Synthetic | (constr1.mods.flags & DefaultParameterized)) :: Nil
val unapplyMeth = {
val unapplyParam = makeSyntheticParameter(tpt = classTypeRef)
val unapplyRHS = if (arity == 0) Literal(Constant(true)) else Ident(unapplyParam.name)
Expand Down Expand Up @@ -403,7 +405,7 @@ object desugar {
// implicit wrapper is typechecked in same scope as constructor, so
// we can reuse the constructor parameters; no derived params are needed.
DefDef(name.toTermName, constrTparams, constrVparamss, classTypeRef, creatorExpr)
.withFlags(Synthetic | Implicit)
.withFlags(Synthetic | Implicit | accessFlags)
.withPos(cdef.pos) :: Nil


Expand Down Expand Up @@ -453,7 +455,7 @@ object desugar {
val clsName = name.moduleClassName
val clsRef = Ident(clsName)
val modul = ValDef(name, clsRef, New(clsRef, Nil))
.withMods(mods | ModuleCreationFlags)
.withMods(mods | ModuleCreationFlags | mods.flags & AccessFlags)
.withPos(mdef.pos)
val ValDef(selfName, selfTpt, _) = tmpl.self
val selfMods = tmpl.self.mods
Expand Down Expand Up @@ -515,7 +517,7 @@ object desugar {
derivedValDef(named, tpt, matchExpr, mods)
case _ =>
val tmpName = ctx.freshName().toTermName
val patFlags = PrivateLocal | Synthetic | (mods.flags & Lazy)
val patFlags = mods.flags & AccessFlags | Synthetic | (mods.flags & Lazy)
val firstDef = ValDef(tmpName, TypeTree(), matchExpr).withFlags(patFlags)
def selector(n: Int) = Select(Ident(tmpName), nme.selectorName(n))
val restDefs =
Expand Down
3 changes: 3 additions & 0 deletions src/dotty/tools/dotc/core/Flags.scala
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,9 @@ object Flags {
/** A lazy or deferred value */
final val LazyOrDeferred = Lazy | Deferred

/** A synthetic or private definition */
final val SyntheticOrPrivate = Synthetic | Private

/** A type parameter or type parameter accessor */
final val TypeParamOrAccessor = TypeParam | TypeParamAccessor

Expand Down
8 changes: 7 additions & 1 deletion src/dotty/tools/dotc/core/Types.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1014,6 +1014,12 @@ object Types {
case _ => List()
}

/** The full parent types, including all type arguments */
def parentsWithArgs(implicit ctx: Context): List[Type] = this match {
case tp: TypeProxy => tp.underlying.parentsWithArgs
case _ => List()
}

/** The first parent of this type, AnyRef if list of parents is empty */
def firstParent(implicit ctx: Context): TypeRef = parents match {
case p :: _ => p
Expand Down Expand Up @@ -2781,7 +2787,7 @@ object Types {
}

/** The parent types with all type arguments */
def instantiatedParents(implicit ctx: Context): List[Type] =
override def parentsWithArgs(implicit ctx: Context): List[Type] =
parents mapConserve { pref =>
((pref: Type) /: pref.classSymbol.typeParams) { (parent, tparam) =>
val targSym = decls.lookup(tparam.name)
Expand Down
4 changes: 2 additions & 2 deletions src/dotty/tools/dotc/printing/Disambiguation.scala
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@ object Disambiguation {
val variants = new mutable.HashMap[String, mutable.ListBuffer[Symbol]]
}

def newPrinter: Context => Printer = {
def newPrinter: Context => RefinedPrinter = {
val state = new State
new Printer(state)(_)
}

class Printer(state: State)(_ctx: Context) extends RefinedPrinter(_ctx) {
private class Printer(state: State)(_ctx: Context) extends RefinedPrinter(_ctx) {
import state._

override def simpleNameString(sym: Symbol): String = {
Expand Down
2 changes: 1 addition & 1 deletion src/dotty/tools/dotc/printing/RefinedPrinter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) {
case ErasedValueType(clazz, underlying) =>
return "ErasedValueType(" ~ toText(clazz.typeRef) ~ ", " ~ toText(underlying) ~ ")"
case tp: ClassInfo =>
return toTextParents(tp.instantiatedParents) ~ "{...}"
return toTextParents(tp.parentsWithArgs) ~ "{...}"
case JavaArrayType(elemtp) =>
return toText(elemtp) ~ "[]"
case tp: SelectionProto =>
Expand Down
3 changes: 3 additions & 0 deletions src/dotty/tools/dotc/reporting/Reporter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,9 @@ trait Reporting { this: Context =>
reporter.report(new Error(msg, pos))
}

def errorOrMigrationWarning(msg: => String, pos: SourcePosition = NoSourcePosition): Unit =
if (ctx.scala2Mode) migrationWarning(msg, pos) else error(msg, pos)

def restrictionError(msg: => String, pos: SourcePosition = NoSourcePosition): Unit =
error(s"Implementation restriction: $msg", pos)

Expand Down
10 changes: 6 additions & 4 deletions src/dotty/tools/dotc/transform/PostTyper.scala
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,10 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisTran
private def transformAnnot(annot: Annotation)(implicit ctx: Context): Annotation =
annot.derivedAnnotation(transformAnnot(annot.tree))

private def transformAnnots(tree: MemberDef)(implicit ctx: Context): Unit =
private def transformMemberDef(tree: MemberDef)(implicit ctx: Context): Unit = {
tree.symbol.transformAnnotations(transformAnnot)
Checking.checkNoPrivateLeaks(tree)
}

private def transformSelect(tree: Select, targs: List[Tree])(implicit ctx: Context): Tree = {
val qual = tree.qualifier
Expand Down Expand Up @@ -172,10 +174,10 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisTran
}
finally parentNews = saved
case tree: DefDef =>
transformAnnots(tree)
transformMemberDef(tree)
superAcc.wrapDefDef(tree)(super.transform(tree).asInstanceOf[DefDef])
case tree: TypeDef =>
transformAnnots(tree)
transformMemberDef(tree)
val sym = tree.symbol
val tree1 =
if (sym.isClass) tree
Expand All @@ -185,7 +187,7 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisTran
}
super.transform(tree1)
case tree: MemberDef =>
transformAnnots(tree)
transformMemberDef(tree)
super.transform(tree)
case tree: New if !inJavaAnnot && !parentNews.contains(tree) =>
Checking.checkInstantiable(tree.tpe, tree.pos)
Expand Down
40 changes: 40 additions & 0 deletions src/dotty/tools/dotc/typer/Checking.scala
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,46 @@ object Checking {
checkNoConflict(Private, Protected)
checkNoConflict(Abstract, Override)
}

/** Check the type signature of the symbol `M` defined by `tree` does not refer
* to a private type or value which is invisible at a point where `M` is still
* visible. As an exception, we allow references to type aliases if the underlying
* type of the alias is not a leak. So type aliases are transparent as far as
* leak testing is concerned. See 997.scala for tests.
*/
def checkNoPrivateLeaks(tree: MemberDef)(implicit ctx: Context): Unit = {
type Errors = List[(String, Position)]
val sym = tree.symbol
val notPrivate = new TypeAccumulator[Errors] {
def accessBoundary(sym: Symbol): Symbol =
if (sym.is(Private)) sym.owner
else if (sym.privateWithin.exists) sym.privateWithin
else if (sym.is(Package)) sym
else accessBoundary(sym.owner)
def apply(errors: Errors, tp: Type): Errors = tp match {
case tp: NamedType =>
val errors1 =
if (tp.symbol.is(Private) &&
!accessBoundary(sym).isContainedIn(tp.symbol.owner)) {
(d"non-private $sym refers to private ${tp.symbol}\n in its type signature ${sym.info}", tree.pos) :: errors
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just call ctx.errorOrMigrationWarning here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because we might end up not issuing the error if a parent is a TypeAlias.

} else foldOver(errors, tp)
if ((errors1 ne errors) && tp.info.isAlias) {
// try to dealias to avoid a leak error
val errors2 = apply(errors, tp.info.bounds.hi)
if (errors2 eq errors) errors2
else errors1
} else errors1
case tp: ClassInfo =>
(apply(errors, tp.prefix) /: tp.parentsWithArgs)(apply)
case _ =>
foldOver(errors, tp)
}
}
if (!sym.is(SyntheticOrPrivate) && sym.owner.isClass) {
val errors = notPrivate(Nil, sym.info)
errors.foreach { case (msg, pos) => ctx.errorOrMigrationWarning(msg, pos) }
}
}
}

trait Checking {
Expand Down
4 changes: 2 additions & 2 deletions src/dotty/tools/dotc/typer/TypeAssigner.scala
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ trait TypeAssigner {
if (tp1.typeSymbol.exists)
return tp1
}
val parentType = info.instantiatedParents.reduceLeft(ctx.typeComparer.andType(_, _))
val parentType = info.parentsWithArgs.reduceLeft(ctx.typeComparer.andType(_, _))
def addRefinement(parent: Type, decl: Symbol) = {
val inherited =
parentType.findMember(decl.name, info.cls.thisType, Private)
Expand Down Expand Up @@ -287,7 +287,7 @@ trait TypeAssigner {
else if (!mix.isEmpty) findMixinSuper(cls.info)
else if (inConstrCall || ctx.erasedTypes) cls.info.firstParent
else {
val ps = cls.classInfo.instantiatedParents
val ps = cls.classInfo.parentsWithArgs
if (ps.isEmpty) defn.AnyType else ps.reduceLeft((x: Type, y: Type) => x & y)
}
tree.withType(SuperType(cls.thisType, owntype))
Expand Down
2 changes: 1 addition & 1 deletion src/dotty/tools/dotc/typer/VarianceChecker.scala
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import config.Printers.variances
* The method should be invoked once for each Template.
*/
object VarianceChecker {
private case class VarianceError(tvar: Symbol, required: Variance)
case class VarianceError(tvar: Symbol, required: Variance)
def check(tree: tpd.Tree)(implicit ctx: Context) =
new VarianceChecker()(ctx).Traverser.traverse(tree)
}
Expand Down
2 changes: 2 additions & 0 deletions test/dotc/tests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,8 @@ class tests extends CompilerTest {
@Test def neg_i803 = compileFile(negDir, "i803", xerrors = 2)
@Test def neg_i866 = compileFile(negDir, "i866", xerrors = 2)
@Test def neg_i974 = compileFile(negDir, "i974", xerrors = 2)
@Test def neg_i997 = compileFile(negDir, "i997", xerrors = 15)
@Test def neg_i997a = compileFile(negDir, "i997a", xerrors = 2)
@Test def neg_i1050 = compileFile(negDir, "i1050", List("-strict"), xerrors = 11)
@Test def neg_i1050a = compileFile(negDir, "i1050a", xerrors = 2)
@Test def neg_i1050c = compileFile(negDir, "i1050c", xerrors = 8)
Expand Down
47 changes: 47 additions & 0 deletions tests/neg/i997.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
class C {

private type T = E
private type Tok = D
private val p: C = new C

def f1(x: T): Unit = () // error
def f1(x: Tok): Unit = () // ok
def f2(x: p.D): Unit = () // error

val v1: T = ??? // error
val v2: p.D = ??? // error

type U1[X <: T] // error
type U2 = T // error

private class E {
def f1ok(x: T): Unit = () // ok
def f2ok(x: p.D): Unit = () // ok

val v1ok: T = ??? // ok
val v2ok: p.D = ??? // ok

type U1ok[X <: T] //ok
type U2ok = T //ok
}

class D extends E { // error
def f1(x: T): Unit = () // error
def f2(x: p.D): Unit = () // error

val v1: T = ??? // error
val v2: p.D = ??? // error

type U1[X <: T] // error
type U2 = T // error
}

class F(x: T) // error

class G private (x: T) // ok

private trait U

class H extends U // error

}
19 changes: 19 additions & 0 deletions tests/neg/i997a.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
class C {

class Super

object O {

private case class CC(x: Int)

private implicit class D(x: Int) extends Super
}

import O._

println(O.CC(1)) // error: CC cannot be accessed

val s: Super = 1 // error: type mismatch


}