From 5006b3ec0e4af08ab3b52b0ff01c3e2b81931f17 Mon Sep 17 00:00:00 2001 From: Jamie Thompson Date: Fri, 8 Jan 2021 15:30:39 +0000 Subject: [PATCH 1/3] fix #10997: check accessible from sealed parent not companion --- .../dotty/tools/dotc/transform/SymUtils.scala | 5 ++-- tests/neg/i10997.scala | 9 +++++++ tests/pos/i10997.scala | 24 +++++++++++++++++++ tests/run/deriving-constructor-order.scala | 2 +- tests/run/deriving.scala | 2 +- 5 files changed, 37 insertions(+), 5 deletions(-) create mode 100644 tests/neg/i10997.scala create mode 100644 tests/pos/i10997.scala diff --git a/compiler/src/dotty/tools/dotc/transform/SymUtils.scala b/compiler/src/dotty/tools/dotc/transform/SymUtils.scala index c2377467a226..0ff860a7fb04 100644 --- a/compiler/src/dotty/tools/dotc/transform/SymUtils.scala +++ b/compiler/src/dotty/tools/dotc/transform/SymUtils.scala @@ -91,7 +91,7 @@ object SymUtils: * It must satisfy the following conditions: * - it has at least one child class or object * - none of its children are anonymous classes - * - all of its children are addressable through a path from its companion object + * - all of its children are addressable through a path from the parent class * - all of its children are generic products or singletons */ def whyNotGenericSum(using Context): String = @@ -99,11 +99,10 @@ object SymUtils: s"it is not a sealed ${self.kindString}" else { val children = self.children - val companion = self.linkedClass def problem(child: Symbol) = { def isAccessible(sym: Symbol): Boolean = - companion.isContainedIn(sym) || sym.is(Module) && isAccessible(sym.owner) + self.isContainedIn(sym) || sym.is(Module) && isAccessible(sym.owner) if (child == self) "it has anonymous or inaccessible subclasses" else if (!isAccessible(child.owner)) i"its child $child is not accessible" diff --git a/tests/neg/i10997.scala b/tests/neg/i10997.scala new file mode 100644 index 000000000000..46040719b690 --- /dev/null +++ b/tests/neg/i10997.scala @@ -0,0 +1,9 @@ +sealed trait Parent + +trait Wrapper { + + case class Foo(x: Int, y: Int, s: String) extends Parent + case class Bar(x: Int, y: Int) extends Parent + + println(summon[deriving.Mirror.Of[Parent]]) // error +} diff --git a/tests/pos/i10997.scala b/tests/pos/i10997.scala new file mode 100644 index 000000000000..fd9562f77d9e --- /dev/null +++ b/tests/pos/i10997.scala @@ -0,0 +1,24 @@ +class Test { + + sealed trait Parent + case class Foo(x: Int, y: Int, s: String) extends Parent + case class Bar(x: Int, y: Int) extends Parent + + println(summon[deriving.Mirror.Of[Parent]]) +} + +object Test2 { + + case class Foo(x: Int, y: Int, s: String) extends i.Parent + case class Bar(x: Int, y: Int) extends i.Parent + + val i = Inner() + + class Inner { + + sealed trait Parent + + println(summon[deriving.Mirror.Of[Parent]]) + } + +} diff --git a/tests/run/deriving-constructor-order.scala b/tests/run/deriving-constructor-order.scala index 89c580971fda..0a3fcde7f84f 100644 --- a/tests/run/deriving-constructor-order.scala +++ b/tests/run/deriving-constructor-order.scala @@ -7,7 +7,7 @@ object Test extends App { case _: T => () } - sealed trait Base1 + sealed trait Base1 // Base1 MUST NOT have a companion here! case class Foo() extends Base1 case object Bar extends Base1 case class Qux(i: Int) extends Base1 diff --git a/tests/run/deriving.scala b/tests/run/deriving.scala index 4e37a2e62f3a..87bd1e088388 100644 --- a/tests/run/deriving.scala +++ b/tests/run/deriving.scala @@ -4,7 +4,7 @@ object T case class A(x: Int, y: Int) extends T case object B extends T -sealed trait U +sealed trait U // U MUST NOT have a companion here! case class C() extends U object Test extends App { From 62abe60f0bd51661e39ab3386b5ea3b022c49193 Mon Sep 17 00:00:00 2001 From: Jamie Thompson Date: Mon, 11 Jan 2021 12:15:20 +0000 Subject: [PATCH 2/3] add test for ordinal and fromProduct --- tests/run/i10997.scala | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 tests/run/i10997.scala diff --git a/tests/run/i10997.scala b/tests/run/i10997.scala new file mode 100644 index 000000000000..30336ca4074e --- /dev/null +++ b/tests/run/i10997.scala @@ -0,0 +1,37 @@ +class ClassWrapper { + + sealed trait Parent + case class Foo(x: Int, y: Int, s: String) extends Parent + case class Bar(x: Int, y: Int) extends Parent + case object Qux extends Parent + + def testcase(): Unit = + + val mirrorParent = summon[deriving.Mirror.Of[Parent]] + val mirrorFoo = summon[deriving.Mirror.Of[Foo]] + val mirrorBar = summon[deriving.Mirror.Of[Bar]] + val mirrorQux = summon[deriving.Mirror.Of[Qux.type]] + + val fooShapedTuple: (Int, Int, String) = (23, 47, "ok") + val barShapedTuple: (Int, Int) = (57, 71) + val quxShapedTuple: EmptyTuple = EmptyTuple + + val foo: Foo = mirrorFoo.fromProduct(fooShapedTuple) + val bar: Bar = mirrorBar.fromProduct(barShapedTuple) + val qux: Qux.type = mirrorQux.fromProduct(quxShapedTuple) + + assert(foo == Foo(23, 47, "ok")) + assert(bar == Bar(57, 71)) + assert(qux == Qux) + + val fooOrd = mirrorParent.ordinal(foo) + val barOrd = mirrorParent.ordinal(bar) + val quxOrd = mirrorParent.ordinal(qux) + + assert(fooOrd == 0) + assert(barOrd == 1) + assert(quxOrd == 2) +} + +@main def Test = + ClassWrapper().testcase() From 6a49bf352a02c2c5a1be25c5c21de2761731d7c8 Mon Sep 17 00:00:00 2001 From: Jamie Thompson Date: Mon, 11 Jan 2021 13:02:50 +0000 Subject: [PATCH 3/3] check children are also accessible in mirror --- .../src/dotty/tools/dotc/transform/SymUtils.scala | 12 +++++++++--- .../tools/dotc/transform/SyntheticMembers.scala | 4 ++-- .../src/dotty/tools/dotc/typer/Synthesizer.scala | 9 +++++---- tests/neg/i10997.scala | 9 +++++++++ 4 files changed, 25 insertions(+), 9 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/SymUtils.scala b/compiler/src/dotty/tools/dotc/transform/SymUtils.scala index 0ff860a7fb04..3bbcdef68932 100644 --- a/compiler/src/dotty/tools/dotc/transform/SymUtils.scala +++ b/compiler/src/dotty/tools/dotc/transform/SymUtils.scala @@ -87,22 +87,28 @@ object SymUtils: def isGenericProduct(using Context): Boolean = whyNotGenericProduct.isEmpty + def useCompanionAsMirror(using Context): Boolean = self.linkedClass.exists && !self.is(Scala2x) + /** Is this a sealed class or trait for which a sum mirror is generated? * It must satisfy the following conditions: * - it has at least one child class or object * - none of its children are anonymous classes * - all of its children are addressable through a path from the parent class + * and also the location of the generated mirror. * - all of its children are generic products or singletons */ - def whyNotGenericSum(using Context): String = + def whyNotGenericSum(declScope: Symbol)(using Context): String = if (!self.is(Sealed)) s"it is not a sealed ${self.kindString}" else { val children = self.children + val companionMirror = self.useCompanionAsMirror + assert(!(companionMirror && (declScope ne self.linkedClass))) def problem(child: Symbol) = { def isAccessible(sym: Symbol): Boolean = - self.isContainedIn(sym) || sym.is(Module) && isAccessible(sym.owner) + (self.isContainedIn(sym) && (companionMirror || declScope.isContainedIn(sym))) + || sym.is(Module) && isAccessible(sym.owner) if (child == self) "it has anonymous or inaccessible subclasses" else if (!isAccessible(child.owner)) i"its child $child is not accessible" @@ -117,7 +123,7 @@ object SymUtils: else children.map(problem).find(!_.isEmpty).getOrElse("") } - def isGenericSum(using Context): Boolean = whyNotGenericSum.isEmpty + def isGenericSum(declScope: Symbol)(using Context): Boolean = whyNotGenericSum(declScope).isEmpty /** If this is a constructor, its owner: otherwise this. */ final def skipConstructor(using Context): Symbol = diff --git a/compiler/src/dotty/tools/dotc/transform/SyntheticMembers.scala b/compiler/src/dotty/tools/dotc/transform/SyntheticMembers.scala index 439b7d0d251e..70d41abe8d63 100644 --- a/compiler/src/dotty/tools/dotc/transform/SyntheticMembers.scala +++ b/compiler/src/dotty/tools/dotc/transform/SyntheticMembers.scala @@ -584,9 +584,9 @@ class SyntheticMembers(thisPhase: DenotTransformer) { if (clazz.is(Module)) { if (clazz.is(Case)) makeSingletonMirror() else if (linked.isGenericProduct) makeProductMirror(linked) - else if (linked.isGenericSum) makeSumMirror(linked) + else if (linked.isGenericSum(clazz)) makeSumMirror(linked) else if (linked.is(Sealed)) - derive.println(i"$linked is not a sum because ${linked.whyNotGenericSum}") + derive.println(i"$linked is not a sum because ${linked.whyNotGenericSum(clazz)}") } else if (impl.removeAttachment(ExtendsSingletonMirror).isDefined) makeSingletonMirror() diff --git a/compiler/src/dotty/tools/dotc/typer/Synthesizer.scala b/compiler/src/dotty/tools/dotc/typer/Synthesizer.scala index 09eb36b5c348..445c130e50fb 100644 --- a/compiler/src/dotty/tools/dotc/typer/Synthesizer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Synthesizer.scala @@ -266,8 +266,10 @@ class Synthesizer(typer: Typer)(using @constructorOnly c: Context): end productMirror private def sumMirror(mirroredType: Type, formal: Type, span: Span)(using Context): Tree = - if mirroredType.classSymbol.isGenericSum then - val cls = mirroredType.classSymbol + val cls = mirroredType.classSymbol + val useCompanion = cls.useCompanionAsMirror + + if cls.isGenericSum(if useCompanion then cls.linkedClass else ctx.owner) then val elemLabels = cls.children.map(c => ConstantType(Constant(c.name.toString))) def solve(sym: Symbol): Type = sym match @@ -318,8 +320,7 @@ class Synthesizer(typer: Typer)(using @constructorOnly c: Context): .refinedWith(tpnme.MirroredElemTypes, TypeAlias(elemsType)) .refinedWith(tpnme.MirroredElemLabels, TypeAlias(TypeOps.nestedPairs(elemLabels))) val mirrorRef = - if cls.linkedClass.exists && !cls.is(Scala2x) - then companionPath(mirroredType, span) + if useCompanion then companionPath(mirroredType, span) else anonymousMirror(monoType, ExtendsSumMirror, span) mirrorRef.cast(mirrorType) else EmptyTree diff --git a/tests/neg/i10997.scala b/tests/neg/i10997.scala index 46040719b690..6d37587d6b10 100644 --- a/tests/neg/i10997.scala +++ b/tests/neg/i10997.scala @@ -7,3 +7,12 @@ trait Wrapper { println(summon[deriving.Mirror.Of[Parent]]) // error } + +class ClassWrapper { + sealed trait Base + case class Foo(x: Int) extends Base +} + +@main def Test = + val cw = new ClassWrapper() + val mirrorParent = summon[deriving.Mirror.Of[cw.Base]] // error: code gen for Mirror can not access each case