From fc043bfb2e1c8fd0a73b87a4c955e3e09f6bf8c0 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 8 Feb 2016 14:35:53 +0100 Subject: Add checking for leaking private definitions First version. Fixes #997. --- src/dotty/tools/dotc/transform/PostTyper.scala | 44 +++++++++++++++++++++++--- 1 file changed, 39 insertions(+), 5 deletions(-) (limited to 'src/dotty/tools/dotc/transform/PostTyper.scala') diff --git a/src/dotty/tools/dotc/transform/PostTyper.scala b/src/dotty/tools/dotc/transform/PostTyper.scala index 14edaa7b5..b4b01d1f3 100644 --- a/src/dotty/tools/dotc/transform/PostTyper.scala +++ b/src/dotty/tools/dotc/transform/PostTyper.scala @@ -127,8 +127,42 @@ 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 = - tree.symbol.transformAnnotations(transformAnnot) + private def transformMemberDef(tree: MemberDef)(implicit ctx: Context): Unit = { + val sym = tree.symbol + sym.transformAnnotations(transformAnnot) + type Errors = List[(String, Position)] + val notPrivate = new TypeAccumulator[Errors] { + def boundary(sym: Symbol): Symbol = + if (sym.is(Private)) sym.owner + else if (sym.privateWithin.exists) sym.privateWithin + else if (sym.is(Package)) sym + else boundary(sym.owner) + def apply(errors: Errors, tp: Type): Errors = tp match { + case tp: NamedType => + val errors1 = + if (tp.symbol.is(Private) && + !boundary(sym).isContainedIn(boundary(tp.symbol))) { + //println(i"sym = $sym, ref = ${tp.symbol}, symb = ${boundary(sym)}, refb = ${boundary(tp.symbol)}") + (d"non-private $sym refers to private ${tp.symbol}\n in its type signature ${sym.info}", tree.pos) :: errors + } + else foldOver(errors, tp) + if ((errors1 ne errors) && tp.info.isAlias) { + 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.typeRef.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) } + } + } private def transformSelect(tree: Select, targs: List[Tree])(implicit ctx: Context): Tree = { val qual = tree.qualifier @@ -172,10 +206,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 @@ -185,7 +219,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) -- cgit v1.2.3 From 54702905111ecc363d6312635415fd9ee7976356 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 8 Feb 2016 14:51:04 +0100 Subject: Move leak detection to Checking Also: include a test that private aliases are transparent. --- src/dotty/tools/dotc/transform/PostTyper.scala | 36 ++--------------------- src/dotty/tools/dotc/typer/Checking.scala | 40 ++++++++++++++++++++++++++ tests/neg/i997.scala | 2 ++ 3 files changed, 44 insertions(+), 34 deletions(-) (limited to 'src/dotty/tools/dotc/transform/PostTyper.scala') diff --git a/src/dotty/tools/dotc/transform/PostTyper.scala b/src/dotty/tools/dotc/transform/PostTyper.scala index b4b01d1f3..8c8eb0978 100644 --- a/src/dotty/tools/dotc/transform/PostTyper.scala +++ b/src/dotty/tools/dotc/transform/PostTyper.scala @@ -128,40 +128,8 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisTran annot.derivedAnnotation(transformAnnot(annot.tree)) private def transformMemberDef(tree: MemberDef)(implicit ctx: Context): Unit = { - val sym = tree.symbol - sym.transformAnnotations(transformAnnot) - type Errors = List[(String, Position)] - val notPrivate = new TypeAccumulator[Errors] { - def boundary(sym: Symbol): Symbol = - if (sym.is(Private)) sym.owner - else if (sym.privateWithin.exists) sym.privateWithin - else if (sym.is(Package)) sym - else boundary(sym.owner) - def apply(errors: Errors, tp: Type): Errors = tp match { - case tp: NamedType => - val errors1 = - if (tp.symbol.is(Private) && - !boundary(sym).isContainedIn(boundary(tp.symbol))) { - //println(i"sym = $sym, ref = ${tp.symbol}, symb = ${boundary(sym)}, refb = ${boundary(tp.symbol)}") - (d"non-private $sym refers to private ${tp.symbol}\n in its type signature ${sym.info}", tree.pos) :: errors - } - else foldOver(errors, tp) - if ((errors1 ne errors) && tp.info.isAlias) { - 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.typeRef.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) } - } + tree.symbol.transformAnnotations(transformAnnot) + Checking.checkNoPrivateLeaks(tree) } private def transformSelect(tree: Select, targs: List[Tree])(implicit ctx: Context): Tree = { diff --git a/src/dotty/tools/dotc/typer/Checking.scala b/src/dotty/tools/dotc/typer/Checking.scala index 437902d05..64aac7d3b 100644 --- a/src/dotty/tools/dotc/typer/Checking.scala +++ b/src/dotty/tools/dotc/typer/Checking.scala @@ -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 + } 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.typeRef.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 { diff --git a/tests/neg/i997.scala b/tests/neg/i997.scala index bfcf5637e..8a21661fb 100644 --- a/tests/neg/i997.scala +++ b/tests/neg/i997.scala @@ -1,9 +1,11 @@ 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 -- cgit v1.2.3