From b6daa2a522e88cfb70776b66301b65ed6d0205f6 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 10 Mar 2016 11:30:34 +0100 Subject: [PATCH 1/6] Make sure lazy accessors in traits are not private. Fixes #1140. Review by @DarkDimius or @smarter. --- src/dotty/tools/dotc/transform/Mixin.scala | 8 ++++++-- tests/run/i1140/A_1.scala | 4 ++++ tests/run/i1140/B_2.scala | 5 +++++ 3 files changed, 15 insertions(+), 2 deletions(-) create mode 100644 tests/run/i1140/A_1.scala create mode 100644 tests/run/i1140/B_2.scala diff --git a/src/dotty/tools/dotc/transform/Mixin.scala b/src/dotty/tools/dotc/transform/Mixin.scala index 32b268fc75f9..fc82caf65aa4 100644 --- a/src/dotty/tools/dotc/transform/Mixin.scala +++ b/src/dotty/tools/dotc/transform/Mixin.scala @@ -98,8 +98,12 @@ class Mixin extends MiniPhaseTransform with SymTransformer { thisTransform => override def runsAfter: Set[Class[_ <: Phase]] = Set(classOf[Erasure]) override def transformSym(sym: SymDenotation)(implicit ctx: Context): SymDenotation = - if (sym.is(Accessor, butNot = Deferred | Lazy) && sym.owner.is(Trait)) - sym.copySymDenotation(initFlags = sym.flags &~ ParamAccessor | Deferred).ensureNotPrivate + if (sym.is(Accessor, butNot = Deferred) && sym.owner.is(Trait)) { + val sym1 = + if (sym is Lazy) sym + else sym.copySymDenotation(initFlags = sym.flags &~ ParamAccessor | Deferred) + sym1.ensureNotPrivate + } else if (sym.isConstructor && sym.owner.is(Trait)) sym.copySymDenotation( name = nme.TRAIT_CONSTRUCTOR, diff --git a/tests/run/i1140/A_1.scala b/tests/run/i1140/A_1.scala new file mode 100644 index 000000000000..e16a8162d993 --- /dev/null +++ b/tests/run/i1140/A_1.scala @@ -0,0 +1,4 @@ +trait A { + val foo = 0 + x + private lazy val x = 1 +} diff --git a/tests/run/i1140/B_2.scala b/tests/run/i1140/B_2.scala new file mode 100644 index 000000000000..aa576c256027 --- /dev/null +++ b/tests/run/i1140/B_2.scala @@ -0,0 +1,5 @@ +object Test extends A { + def main(args: Array[String]): Unit = { + assert(foo == 1) + } +} From 9ae558a7a7060ac7dfc6ef3fc482e8eda66d33e3 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 10 Mar 2016 18:39:54 +0100 Subject: [PATCH 2/6] More detailed reporting in TreeChecker ... when definitions are missing. --- src/dotty/tools/dotc/transform/TreeChecker.scala | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/dotty/tools/dotc/transform/TreeChecker.scala b/src/dotty/tools/dotc/transform/TreeChecker.scala index 150a632a1476..a260963e917a 100644 --- a/src/dotty/tools/dotc/transform/TreeChecker.scala +++ b/src/dotty/tools/dotc/transform/TreeChecker.scala @@ -282,7 +282,10 @@ class TreeChecker extends Phase with SymTransformer { val symbolsNotDefined = cls.classInfo.decls.toSet.filter(isNonMagicalMethod) -- impl.body.map(_.symbol) - constr.symbol - assert(symbolsNotDefined.isEmpty, i" $cls tree does not define methods: $symbolsNotDefined") + assert(symbolsNotDefined.isEmpty, + i" $cls tree does not define methods: ${symbolsNotDefined.toList}%, %\n" + + i"expected: ${cls.classInfo.decls.toSet.filter(isNonMagicalMethod).toList}%, %\n" + + i"defined: ${impl.body.map(_.symbol)}%, %") super.typedClassDef(cdef, cls) } From df5fcefac142f6ab65a93c33ec9179380025a492 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 10 Mar 2016 18:41:03 +0100 Subject: [PATCH 3/6] Fix and simplify initializer Initializer was needlessly complex and did not work anymore for lazy vals (for them, we implicitly made use of the fact that the initializer would find the symbol itself. But after name mangling that logic would break down. --- src/dotty/tools/dotc/transform/Mixin.scala | 24 ++++++++++++---------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/dotty/tools/dotc/transform/Mixin.scala b/src/dotty/tools/dotc/transform/Mixin.scala index fc82caf65aa4..3ec2aedcbaec 100644 --- a/src/dotty/tools/dotc/transform/Mixin.scala +++ b/src/dotty/tools/dotc/transform/Mixin.scala @@ -112,17 +112,19 @@ class Mixin extends MiniPhaseTransform with SymTransformer { thisTransform => sym private def initializer(sym: Symbol)(implicit ctx: Context): TermSymbol = { - val initName = if(!sym.is(Lazy)) InitializerName(sym.name.asTermName) else sym.name.asTermName - sym.owner.info.decl(initName).suchThat(_.is(Lazy) == sym.is(Lazy)).symbol - .orElse( - ctx.newSymbol( - sym.owner, - initName, - Protected | Synthetic | Method, - sym.info, - coord = sym.symbol.coord).enteredAfter(thisTransform)) - .asTerm - } + if (sym is Lazy) sym + else { + val initName = InitializerName(sym.name.asTermName) + sym.owner.info.decl(initName).symbol + .orElse( + ctx.newSymbol( + sym.owner, + initName, + Protected | Synthetic | Method, + sym.info, + coord = sym.symbol.coord).enteredAfter(thisTransform)) + } + }.asTerm override def transformTemplate(impl: Template)(implicit ctx: Context, info: TransformerInfo) = { val cls = impl.symbol.owner.asClass From d500b1d6caaafcc9258263c98dc5250f662410c7 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 13 Mar 2016 15:06:27 +0100 Subject: [PATCH 4/6] Propagate source file to synthetic companions --- src/dotty/tools/dotc/transform/FirstTransform.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dotty/tools/dotc/transform/FirstTransform.scala b/src/dotty/tools/dotc/transform/FirstTransform.scala index 6b13d46b282b..37ae1d94e8bc 100644 --- a/src/dotty/tools/dotc/transform/FirstTransform.scala +++ b/src/dotty/tools/dotc/transform/FirstTransform.scala @@ -124,7 +124,7 @@ class FirstTransform extends MiniPhaseTransform with IdentityDenotTransformer wi private def newCompanion(name: TermName, forClass: Symbol)(implicit ctx: Context) = { val modul = ctx.newCompleteModuleSymbol(forClass.owner, name, Synthetic, Synthetic, - defn.ObjectType :: Nil, Scopes.newScope) + defn.ObjectType :: Nil, Scopes.newScope, assocFile = forClass.asClass.assocFile) val mc = modul.moduleClass val mcComp = ctx.synthesizeCompanionMethod(nme.COMPANION_CLASS_METHOD, forClass, mc) From 9daf976a2040eaef2699cdaef6360fcbf8af6aa0 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 13 Mar 2016 15:08:09 +0100 Subject: [PATCH 5/6] Assert that ensureNotPrivate does not break on separate compilation When ensureNotPrivate changes the status of a formerly private declaration, assert that the reference to the declaration is in the same compilation unit, as otherwise the nehavior would be different under separate compilation. --- src/dotty/tools/dotc/transform/ExpandPrivate.scala | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/dotty/tools/dotc/transform/ExpandPrivate.scala b/src/dotty/tools/dotc/transform/ExpandPrivate.scala index f44be39584bb..a6f2034783cf 100644 --- a/src/dotty/tools/dotc/transform/ExpandPrivate.scala +++ b/src/dotty/tools/dotc/transform/ExpandPrivate.scala @@ -57,8 +57,11 @@ class ExpandPrivate extends MiniPhaseTransform with IdentityDenotTransformer { t * static members of the companion class, we should tighten the condition below. */ private def ensurePrivateAccessible(d: SymDenotation)(implicit ctx: Context) = - if (d.is(PrivateTerm) && d.owner != ctx.owner.enclosingClass) + if (d.is(PrivateTerm) && d.owner != ctx.owner.enclosingClass) { + assert(d.symbol.sourceFile == ctx.source.file, + i"private ${d.symbol.showLocated} in ${d.symbol.sourceFile} accessed from ${ctx.owner.showLocated} in ${ctx.source.file}") d.ensureNotPrivate.installAfter(thisTransform) + } override def transformIdent(tree: Ident)(implicit ctx: Context, info: TransformerInfo) = { ensurePrivateAccessible(tree.symbol) From 6b983e39ff7880994c6edfec7e79a151f83e83f3 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 13 Mar 2016 15:08:39 +0100 Subject: [PATCH 6/6] Make Synthetic a FormStart flag. Not needed in the end for this patch, but anyway a good idea. --- src/dotty/tools/dotc/core/Flags.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dotty/tools/dotc/core/Flags.scala b/src/dotty/tools/dotc/core/Flags.scala index ebf4c637add6..f866621f258d 100644 --- a/src/dotty/tools/dotc/core/Flags.scala +++ b/src/dotty/tools/dotc/core/Flags.scala @@ -448,7 +448,7 @@ object Flags { final val FromStartFlags = AccessFlags | Module | Package | Deferred | Final | MethodOrHKCommon | Param | ParamAccessor | Scala2ExistentialCommon | InSuperCall | Touched | JavaStatic | CovariantOrOuter | ContravariantOrLabel | ExpandedName | AccessorOrSealed | - CaseAccessorOrBaseTypeArg | Fresh | Frozen | Erroneous | ImplicitCommon | Permanent | + CaseAccessorOrBaseTypeArg | Fresh | Frozen | Erroneous | ImplicitCommon | Permanent | Synthetic | LazyOrTrait | SuperAccessorOrScala2x | SelfNameOrImplClass assert(FromStartFlags.isTermFlags && FromStartFlags.isTypeFlags)