From 6a93dd0dea9833ecba70d09a414a650227c03fc9 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 8 Feb 2016 13:52:28 +0100 Subject: Copy access flags to derived definitions during desugaring Previously, some definitions were too public, others too private. --- src/dotty/tools/dotc/ast/Desugar.scala | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'src/dotty') diff --git a/src/dotty/tools/dotc/ast/Desugar.scala b/src/dotty/tools/dotc/ast/Desugar.scala index 991940f10..2b0d30c7f 100644 --- a/src/dotty/tools/dotc/ast/Desugar.scala +++ b/src/dotty/tools/dotc/ast/Desugar.scala @@ -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) @@ -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 { @@ -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. @@ -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) @@ -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 @@ -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 @@ -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 = -- cgit v1.2.3 From eb1908a9f2c61895cabe70c0ac0ebbe8ef14fcea Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 8 Feb 2016 14:34:10 +0100 Subject: New utility method: Reporter#errorOrMigrationWarning --- src/dotty/tools/dotc/reporting/Reporter.scala | 3 +++ 1 file changed, 3 insertions(+) (limited to 'src/dotty') diff --git a/src/dotty/tools/dotc/reporting/Reporter.scala b/src/dotty/tools/dotc/reporting/Reporter.scala index d6e8199d8..272b1880f 100644 --- a/src/dotty/tools/dotc/reporting/Reporter.scala +++ b/src/dotty/tools/dotc/reporting/Reporter.scala @@ -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) -- cgit v1.2.3 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/core/Flags.scala | 3 ++ src/dotty/tools/dotc/core/Types.scala | 9 ++++++ src/dotty/tools/dotc/transform/PostTyper.scala | 44 ++++++++++++++++++++++--- test/dotc/tests.scala | 2 ++ tests/neg/i997.scala | 45 ++++++++++++++++++++++++++ 5 files changed, 98 insertions(+), 5 deletions(-) create mode 100644 tests/neg/i997.scala (limited to 'src/dotty') diff --git a/src/dotty/tools/dotc/core/Flags.scala b/src/dotty/tools/dotc/core/Flags.scala index 02b341649..d1f562f72 100644 --- a/src/dotty/tools/dotc/core/Flags.scala +++ b/src/dotty/tools/dotc/core/Flags.scala @@ -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 diff --git a/src/dotty/tools/dotc/core/Types.scala b/src/dotty/tools/dotc/core/Types.scala index 8595da640..16287c827 100644 --- a/src/dotty/tools/dotc/core/Types.scala +++ b/src/dotty/tools/dotc/core/Types.scala @@ -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.parents + 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 @@ -2780,6 +2786,9 @@ object Types { parentsCache } + override def parentsWithArgs(implicit ctx: Context): List[Type] = + parents.map(p => typeRef.baseTypeWithArgs(p.symbol)) + /** The parent types with all type arguments */ def instantiatedParents(implicit ctx: Context): List[Type] = parents mapConserve { pref => 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) diff --git a/test/dotc/tests.scala b/test/dotc/tests.scala index 421846ca2..573436733 100644 --- a/test/dotc/tests.scala +++ b/test/dotc/tests.scala @@ -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, "i997a", 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) diff --git a/tests/neg/i997.scala b/tests/neg/i997.scala new file mode 100644 index 000000000..bfcf5637e --- /dev/null +++ b/tests/neg/i997.scala @@ -0,0 +1,45 @@ +class C { + + private type T = E + private val p: C = new C + + 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 + + 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 + +} -- cgit v1.2.3 From 19026b87c47a5aeca98387f39f6d59fae5bec846 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 8 Feb 2016 14:37:47 +0100 Subject: Fix two private leaks in dotty compiler itself. --- src/dotty/tools/dotc/printing/Disambiguation.scala | 4 ++-- src/dotty/tools/dotc/typer/VarianceChecker.scala | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'src/dotty') diff --git a/src/dotty/tools/dotc/printing/Disambiguation.scala b/src/dotty/tools/dotc/printing/Disambiguation.scala index 29b290f03..aa3fae2de 100644 --- a/src/dotty/tools/dotc/printing/Disambiguation.scala +++ b/src/dotty/tools/dotc/printing/Disambiguation.scala @@ -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 = { diff --git a/src/dotty/tools/dotc/typer/VarianceChecker.scala b/src/dotty/tools/dotc/typer/VarianceChecker.scala index bbe391726..b257ee192 100644 --- a/src/dotty/tools/dotc/typer/VarianceChecker.scala +++ b/src/dotty/tools/dotc/typer/VarianceChecker.scala @@ -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) } -- 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') 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 From 97e261d17bb260cd0db4708199118d7039e442fa Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 10 Feb 2016 08:39:46 +0100 Subject: Merge parentsWithArgs and instantiatedParents --- src/dotty/tools/dotc/core/Types.scala | 7 ++----- src/dotty/tools/dotc/printing/RefinedPrinter.scala | 2 +- src/dotty/tools/dotc/typer/Checking.scala | 2 +- src/dotty/tools/dotc/typer/TypeAssigner.scala | 4 ++-- 4 files changed, 6 insertions(+), 9 deletions(-) (limited to 'src/dotty') diff --git a/src/dotty/tools/dotc/core/Types.scala b/src/dotty/tools/dotc/core/Types.scala index 16287c827..9b58f8eeb 100644 --- a/src/dotty/tools/dotc/core/Types.scala +++ b/src/dotty/tools/dotc/core/Types.scala @@ -1016,7 +1016,7 @@ object Types { /** The full parent types, including all type arguments */ def parentsWithArgs(implicit ctx: Context): List[Type] = this match { - case tp: TypeProxy => tp.underlying.parents + case tp: TypeProxy => tp.underlying.parentsWithArgs case _ => List() } @@ -2786,11 +2786,8 @@ object Types { parentsCache } - override def parentsWithArgs(implicit ctx: Context): List[Type] = - parents.map(p => typeRef.baseTypeWithArgs(p.symbol)) - /** 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) diff --git a/src/dotty/tools/dotc/printing/RefinedPrinter.scala b/src/dotty/tools/dotc/printing/RefinedPrinter.scala index 204fc95f0..068ed4daa 100644 --- a/src/dotty/tools/dotc/printing/RefinedPrinter.scala +++ b/src/dotty/tools/dotc/printing/RefinedPrinter.scala @@ -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 => diff --git a/src/dotty/tools/dotc/typer/Checking.scala b/src/dotty/tools/dotc/typer/Checking.scala index 64aac7d3b..beb8fc931 100644 --- a/src/dotty/tools/dotc/typer/Checking.scala +++ b/src/dotty/tools/dotc/typer/Checking.scala @@ -342,7 +342,7 @@ object Checking { else errors1 } else errors1 case tp: ClassInfo => - (apply(errors, tp.prefix) /: tp.typeRef.parentsWithArgs)(apply) + (apply(errors, tp.prefix) /: tp.parentsWithArgs)(apply) case _ => foldOver(errors, tp) } diff --git a/src/dotty/tools/dotc/typer/TypeAssigner.scala b/src/dotty/tools/dotc/typer/TypeAssigner.scala index ac46ee723..07cc3c8d6 100644 --- a/src/dotty/tools/dotc/typer/TypeAssigner.scala +++ b/src/dotty/tools/dotc/typer/TypeAssigner.scala @@ -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) @@ -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)) -- cgit v1.2.3