Skip to content

Commit

Permalink
Merge pull request #15231 from dotty-staging/fix-15222
Browse files Browse the repository at this point in the history
fix #15222 recursively check for product ctor accessibility
  • Loading branch information
bishabosha authored May 27, 2022
2 parents dbbb8b7 + f1b95a5 commit 001bfc3
Show file tree
Hide file tree
Showing 9 changed files with 164 additions and 85 deletions.
38 changes: 28 additions & 10 deletions compiler/src/dotty/tools/dotc/transform/SymUtils.scala
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,26 @@ object SymUtils:
* parameter section.
*/
def whyNotGenericProduct(using Context): String =
/** for a case class, if it will have an anonymous mirror,
* check that its constructor can be accessed
* from the calling scope.
*/
def canAccessCtor: Boolean =
def isAccessible(sym: Symbol): Boolean = ctx.owner.isContainedIn(sym)
def isSub(sym: Symbol): Boolean = ctx.owner.ownersIterator.exists(_.derivesFrom(sym))
val ctor = self.primaryConstructor
(!ctor.isOneOf(Private | Protected) || isSub(self)) // we cant access the ctor because we do not extend cls
&& (!ctor.privateWithin.exists || isAccessible(ctor.privateWithin)) // check scope is compatible


def companionMirror = self.useCompanionAsProductMirror
if (!self.is(CaseClass)) "it is not a case class"
else if (self.is(Abstract)) "it is an abstract class"
else if (self.primaryConstructor.info.paramInfoss.length != 1) "it takes more than one parameter list"
else if (isDerivedValueClass(self)) "it is a value class"
else if (!(companionMirror || canAccessCtor)) s"the constructor of $self is innaccessible from the calling scope."
else ""
end whyNotGenericProduct

def isGenericProduct(using Context): Boolean = whyNotGenericProduct.isEmpty

Expand Down Expand Up @@ -120,6 +135,9 @@ object SymUtils:
self.isOneOf(FinalOrInline, butNot = Mutable)
&& (!self.is(Method) || self.is(Accessor))

def useCompanionAsProductMirror(using Context): Boolean =
self.linkedClass.exists && !self.is(Scala2x) && !self.linkedClass.is(Case)

def useCompanionAsSumMirror(using Context): Boolean =
def companionExtendsSum(using Context): Boolean =
self.linkedClass.isSubClass(defn.Mirror_SumClass)
Expand All @@ -145,39 +163,39 @@ object SymUtils:
* and also the location of the generated mirror.
* - all of its children are generic products, singletons, or generic sums themselves.
*/
def whyNotGenericSum(declScope: Symbol)(using Context): String =
def whyNotGenericSum(using Context): String =
if (!self.is(Sealed))
s"it is not a sealed ${self.kindString}"
else if (!self.isOneOf(AbstractOrTrait))
"it is not an abstract class"
else {
val children = self.children
val companionMirror = self.useCompanionAsSumMirror
assert(!(companionMirror && (declScope ne self.linkedClass)))
def problem(child: Symbol) = {

def isAccessible(sym: Symbol): Boolean =
(self.isContainedIn(sym) && (companionMirror || declScope.isContainedIn(sym)))
(self.isContainedIn(sym) && (companionMirror || ctx.owner.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"
else if (!child.isClass) ""
else if (!child.isClass) "" // its a singleton enum value
else {
val s = child.whyNotGenericProduct
if (s.isEmpty) s
else if (child.is(Sealed)) {
val s = child.whyNotGenericSum(if child.useCompanionAsSumMirror then child.linkedClass else ctx.owner)
if (s.isEmpty) s
if s.isEmpty then s
else if child.is(Sealed) then
val s = child.whyNotGenericSum
if s.isEmpty then s
else i"its child $child is not a generic sum because $s"
} else i"its child $child is not a generic product because $s"
else
i"its child $child is not a generic product because $s"
}
}
if (children.isEmpty) "it does not have subclasses"
else children.map(problem).find(!_.isEmpty).getOrElse("")
}

def isGenericSum(declScope: Symbol)(using Context): Boolean = whyNotGenericSum(declScope).isEmpty
def isGenericSum(using Context): Boolean = whyNotGenericSum.isEmpty

/** If this is a constructor, its owner: otherwise this. */
final def skipConstructor(using Context): Symbol =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -594,9 +594,9 @@ class SyntheticMembers(thisPhase: DenotTransformer) {
if (clazz.is(Module)) {
if (clazz.is(Case)) makeSingletonMirror()
else if (linked.isGenericProduct) makeProductMirror(linked)
else if (linked.isGenericSum(clazz)) makeSumMirror(linked)
else if (linked.isGenericSum) makeSumMirror(linked)
else if (linked.is(Sealed))
derive.println(i"$linked is not a sum because ${linked.whyNotGenericSum(clazz)}")
derive.println(i"$linked is not a sum because ${linked.whyNotGenericSum}")
}
else if (impl.removeAttachment(ExtendsSingletonMirror).isDefined)
makeSingletonMirror()
Expand Down
130 changes: 61 additions & 69 deletions compiler/src/dotty/tools/dotc/typer/Synthesizer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class Synthesizer(typer: Typer)(using @constructorOnly c: Context):
/** Handlers to synthesize implicits for special types */
type SpecialHandler = (Type, Span) => Context ?=> TreeWithErrors
private type SpecialHandlers = List[(ClassSymbol, SpecialHandler)]

val synthesizedClassTag: SpecialHandler = (formal, span) =>
formal.argInfos match
case arg :: Nil =>
Expand Down Expand Up @@ -278,28 +278,15 @@ class Synthesizer(typer: Typer)(using @constructorOnly c: Context):

private def productMirror(mirroredType: Type, formal: Type, span: Span)(using Context): TreeWithErrors =

/** do all parts match the class symbol? */
def acceptable(tp: Type, cls: Symbol): Boolean = tp match
case tp: HKTypeLambda if tp.resultType.isInstanceOf[HKTypeLambda] => false
case tp: TypeProxy => acceptable(tp.underlying, cls)
case OrType(tp1, tp2) => acceptable(tp1, cls) && acceptable(tp2, cls)
case _ => tp.classSymbol eq cls

/** for a case class, if it will have an anonymous mirror,
* check that its constructor can be accessed
* from the calling scope.
*/
def canAccessCtor(cls: Symbol): Boolean =
!genAnonyousMirror(cls) || {
def isAccessible(sym: Symbol): Boolean = ctx.owner.isContainedIn(sym)
def isSub(sym: Symbol): Boolean = ctx.owner.ownersIterator.exists(_.derivesFrom(sym))
val ctor = cls.primaryConstructor
(!ctor.isOneOf(Private | Protected) || isSub(cls)) // we cant access the ctor because we do not extend cls
&& (!ctor.privateWithin.exists || isAccessible(ctor.privateWithin)) // check scope is compatible
}

def genAnonyousMirror(cls: Symbol): Boolean =
cls.is(Scala2x) || cls.linkedClass.is(Case)
def whyNotAcceptableType(tp: Type, cls: Symbol): String = tp match
case tp: HKTypeLambda if tp.resultType.isInstanceOf[HKTypeLambda] =>
i"its subpart $tp is not a supported kind (either `*` or `* -> *`)"
case tp: TypeProxy => whyNotAcceptableType(tp.underlying, cls)
case OrType(tp1, tp2) =>
Seq(tp1, tp2).map(whyNotAcceptableType(_, cls)).find(_.nonEmpty).getOrElse("")
case _ =>
if tp.classSymbol eq cls then ""
else i"a subpart reduces to the more precise ${tp.classSymbol}, expected $cls"

def makeProductMirror(cls: Symbol): TreeWithErrors =
val accessors = cls.caseAccessors.filterNot(_.isAllOf(PrivateLocal))
Expand All @@ -318,61 +305,63 @@ class Synthesizer(typer: Typer)(using @constructorOnly c: Context):
.refinedWith(tpnme.MirroredElemTypes, TypeAlias(elemsType))
.refinedWith(tpnme.MirroredElemLabels, TypeAlias(elemsLabels))
val mirrorRef =
if (genAnonyousMirror(cls)) anonymousMirror(monoType, ExtendsProductMirror, span)
else companionPath(mirroredType, span)
if cls.useCompanionAsProductMirror then companionPath(mirroredType, span)
else anonymousMirror(monoType, ExtendsProductMirror, span)
withNoErrors(mirrorRef.cast(mirrorType))
end makeProductMirror

def getError(cls: Symbol): String =
val reason = if !cls.isGenericProduct then
i"because ${cls.whyNotGenericProduct}"
else if !canAccessCtor(cls) then
i"because the constructor of $cls is innaccessible from the calling scope."
else
""
i"$cls is not a generic product $reason"
end getError
/** widen TermRef to see if they are an alias to an enum singleton */
def isEnumSingletonRef(tp: Type)(using Context): Boolean = tp match
case tp: TermRef =>
val sym = tp.termSymbol
sym.isEnumCase || (!tp.isOverloaded && isEnumSingletonRef(tp.underlying.widenExpr))
case _ => false

mirroredType match
case AndType(tp1, tp2) =>
orElse(productMirror(tp1, formal, span), productMirror(tp2, formal, span))
case _ =>
if mirroredType.termSymbol.is(CaseVal) then
val module = mirroredType.termSymbol
val modulePath = pathFor(mirroredType).withSpan(span)
if module.info.classSymbol.is(Scala2x) then
val mirrorType = mirrorCore(defn.Mirror_SingletonProxyClass, mirroredType, mirroredType, module.name, formal)
val mirrorRef = New(defn.Mirror_SingletonProxyClass.typeRef, modulePath :: Nil)
val cls = mirroredType.classSymbol
if isEnumSingletonRef(mirroredType) || cls.isAllOf(Case | Module) then
val (singleton, singletonRef) =
if mirroredType.termSymbol.exists then (mirroredType.termSymbol, mirroredType)
else (cls.sourceModule, cls.sourceModule.reachableTermRef)
val singletonPath = pathFor(singletonRef).withSpan(span)
if singleton.info.classSymbol.is(Scala2x) then // could be Scala 3 alias of Scala 2 case object.
val mirrorType = mirrorCore(defn.Mirror_SingletonProxyClass, mirroredType, mirroredType, singleton.name, formal)
val mirrorRef = New(defn.Mirror_SingletonProxyClass.typeRef, singletonPath :: Nil)
withNoErrors(mirrorRef.cast(mirrorType))
else
val mirrorType = mirrorCore(defn.Mirror_SingletonClass, mirroredType, mirroredType, module.name, formal)
withNoErrors(modulePath.cast(mirrorType))
val mirrorType = mirrorCore(defn.Mirror_SingletonClass, mirroredType, mirroredType, singleton.name, formal)
withNoErrors(singletonPath.cast(mirrorType))
else
val cls = mirroredType.classSymbol
if acceptable(mirroredType, cls)
&& cls.isGenericProduct
&& canAccessCtor(cls)
then
makeProductMirror(cls)
else
(EmptyTree, List(getError(cls)))
val acceptableMsg = whyNotAcceptableType(mirroredType, cls)
if acceptableMsg.isEmpty then
if cls.isGenericProduct then makeProductMirror(cls)
else withErrors(i"$cls is not a generic product because ${cls.whyNotGenericProduct}")
else withErrors(i"type $mirroredType is not a generic product because $acceptableMsg")
end productMirror

private def sumMirror(mirroredType: Type, formal: Type, span: Span)(using Context): TreeWithErrors =

val cls = mirroredType.classSymbol
val useCompanion = cls.useCompanionAsSumMirror
val declScope = if useCompanion then cls.linkedClass else ctx.owner
val clsIsGenericSum = cls.isGenericSum(declScope)

def acceptable(tp: Type): Boolean = tp match
case tp: TermRef => false
case tp: HKTypeLambda if tp.resultType.isInstanceOf[HKTypeLambda] => false
case tp: TypeProxy => acceptable(tp.underlying)
case OrType(tp1, tp2) => acceptable(tp1) && acceptable(tp2)
case _ => tp.classSymbol eq cls

if acceptable(mirroredType) && clsIsGenericSum then
val clsIsGenericSum = cls.isGenericSum

def whyNotAcceptableType(tp: Type): String = tp match
case tp: TermRef => i"its subpart $tp is a term reference"
case tp: HKTypeLambda if tp.resultType.isInstanceOf[HKTypeLambda] =>
i"its subpart $tp is not a supported kind (either `*` or `* -> *`)"
case tp: TypeProxy => whyNotAcceptableType(tp.underlying)
case OrType(tp1, tp2) =>
Seq(tp1, tp2).map(whyNotAcceptableType).find(_.nonEmpty).getOrElse("")
case _ =>
if tp.classSymbol eq cls then ""
else i"a subpart reduces to the more precise ${tp.classSymbol}, expected $cls"


val acceptableMsg = whyNotAcceptableType(mirroredType)

if acceptableMsg.isEmpty && clsIsGenericSum then
val elemLabels = cls.children.map(c => ConstantType(Constant(c.name.toString)))

def solve(sym: Symbol): Type = sym match
Expand Down Expand Up @@ -423,12 +412,14 @@ class Synthesizer(typer: Typer)(using @constructorOnly c: Context):
.refinedWith(tpnme.MirroredElemTypes, TypeAlias(elemsType))
.refinedWith(tpnme.MirroredElemLabels, TypeAlias(TypeOps.nestedPairs(elemLabels)))
val mirrorRef =
if useCompanion then companionPath(mirroredType, span)
if cls.useCompanionAsSumMirror then companionPath(mirroredType, span)
else anonymousMirror(monoType, ExtendsSumMirror, span)
withNoErrors(mirrorRef.cast(mirrorType))
else if acceptableMsg.nonEmpty then
withErrors(i"type $mirroredType is not a generic sum because $acceptableMsg")
else if !clsIsGenericSum then
(EmptyTree, List(i"$cls is not a generic sum because ${cls.whyNotGenericSum(declScope)}"))
else
withErrors(i"$cls is not a generic sum because ${cls.whyNotGenericSum}")
else
EmptyTreeNoError
end sumMirror

Expand Down Expand Up @@ -595,7 +586,7 @@ class Synthesizer(typer: Typer)(using @constructorOnly c: Context):
tp.baseType(cls)
val base = baseWithRefinements(formal)
val result =
if (base <:< formal.widenExpr)
if (base <:< formal.widenExpr)
// With the subtype test we enforce that the searched type `formal` is of the right form
handler(base, span)
else EmptyTreeNoError
Expand All @@ -609,19 +600,20 @@ end Synthesizer

object Synthesizer:

/** Tuple used to store the synthesis result with a list of errors. */
/** Tuple used to store the synthesis result with a list of errors. */
type TreeWithErrors = (Tree, List[String])
private def withNoErrors(tree: Tree): TreeWithErrors = (tree, List.empty)
private def withErrors(errors: String*): TreeWithErrors = (EmptyTree, errors.toList)

private val EmptyTreeNoError: TreeWithErrors = withNoErrors(EmptyTree)

private def orElse(treeWithErrors1: TreeWithErrors, treeWithErrors2: => TreeWithErrors): TreeWithErrors = treeWithErrors1 match
case (tree, errors) if tree eq genericEmptyTree =>
case (tree, errors) if tree eq genericEmptyTree =>
val (tree2, errors2) = treeWithErrors2
(tree2, errors ::: errors2)
case _ => treeWithErrors1

private def clearErrorsIfNotEmpty(treeWithErrors: TreeWithErrors) = treeWithErrors match
private def clearErrorsIfNotEmpty(treeWithErrors: TreeWithErrors) = treeWithErrors match
case (tree, _) if tree eq genericEmptyTree => treeWithErrors
case (tree, _) => withNoErrors(tree)

Expand Down
4 changes: 2 additions & 2 deletions tests/neg/i14025.check
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
-- Error: tests/neg/i14025.scala:1:88 ----------------------------------------------------------------------------------
1 |val foo = summon[deriving.Mirror.Product { type MirroredType = [X] =>> [Y] =>> (X, Y) }] // error
| ^
|No given instance of type deriving.Mirror.Product{MirroredType[X] = [Y] =>> (X, Y)} was found for parameter x of method summon in object Predef. Failed to synthesize an instance of type deriving.Mirror.Product{MirroredType[X] = [Y] =>> (X, Y)}: class Tuple2 is not a generic product
|No given instance of type deriving.Mirror.Product{MirroredType[X] = [Y] =>> (X, Y)} was found for parameter x of method summon in object Predef. Failed to synthesize an instance of type deriving.Mirror.Product{MirroredType[X] = [Y] =>> (X, Y)}: type [X] =>> [Y] =>> (X, Y) is not a generic product because its subpart [X] =>> [Y] =>> (X, Y) is not a supported kind (either `*` or `* -> *`)
-- Error: tests/neg/i14025.scala:2:90 ----------------------------------------------------------------------------------
2 |val bar = summon[deriving.Mirror.Sum { type MirroredType = [X] =>> [Y] =>> List[(X, Y)] }] // error
| ^
|No given instance of type deriving.Mirror.Sum{MirroredType[X] = [Y] =>> List[(X, Y)]} was found for parameter x of method summon in object Predef
|No given instance of type deriving.Mirror.Sum{MirroredType[X] = [Y] =>> List[(X, Y)]} was found for parameter x of method summon in object Predef. Failed to synthesize an instance of type deriving.Mirror.Sum{MirroredType[X] = [Y] =>> List[(X, Y)]}: type [X] =>> [Y] =>> List[(X, Y)] is not a generic sum because its subpart [X] =>> [Y] =>> List[(X, Y)] is not a supported kind (either `*` or `* -> *`)
4 changes: 2 additions & 2 deletions tests/neg/i14823.check
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@
8 |val baz = summon[Mirror.Of[SubA[Int] | SubB[Int]]] // error
| ^
|No given instance of type deriving.Mirror.Of[SubA[Int] | SubB[Int]] was found for parameter x of method summon in object Predef. Failed to synthesize an instance of type deriving.Mirror.Of[SubA[Int] | SubB[Int]]:
| * class Cov is not a generic product
| * class Cov is not a generic sum because it is not a sealed class
| * type SubA[Int] | SubB[Int] is not a generic product because a subpart reduces to the more precise class SubA, expected class Cov
| * type SubA[Int] | SubB[Int] is not a generic sum because a subpart reduces to the more precise class SubA, expected class Cov
26 changes: 26 additions & 0 deletions tests/neg/prod-mirror-inacessible-ctor.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import scala.deriving.Mirror

package lib {
sealed trait Foo
object Foo // normally, would cache a mirror if one exists.
case class Bar private[lib] () extends Foo
case object Bar // force mirror for Bar to be anonymous.


object CallSiteSucceed {
val mFoo = summon[Mirror.SumOf[lib.Foo]] // ok
val mBar = summon[Mirror.ProductOf[lib.Bar]] // ok
}

}

package app {

object MustFail {
// we are outsite of accessible scope for Bar's ctor, so this should fail.

val mFoo = summon[Mirror.SumOf[lib.Foo]] // error
val mBar = summon[Mirror.ProductOf[lib.Bar]] // error
}

}
22 changes: 22 additions & 0 deletions tests/run/i15234.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import scala.deriving.Mirror

package lib {
enum Foo:
case A

case object Bar
}

package app {
object Foo:
val A: lib.Foo.A.type = lib.Foo.A
val Bar: lib.Bar.type = lib.Bar
}


@main def Test =
assert(summon[Mirror.Of[scala.Nil.type]].fromProduct(EmptyTuple) == Nil) // alias scala 2 defined
assert(summon[Mirror.Of[lib.Foo.A.type]].fromProduct(EmptyTuple) == lib.Foo.A) // real mirror
assert(summon[Mirror.Of[lib.Bar.type]].fromProduct(EmptyTuple) == lib.Bar) // real mirror
assert(summon[Mirror.Of[app.Foo.A.type]].fromProduct(EmptyTuple) == lib.Foo.A) // alias mirror
assert(summon[Mirror.Of[app.Bar.type]].fromProduct(EmptyTuple) == lib.Bar) // alias mirror
15 changes: 15 additions & 0 deletions tests/run/prod-mirror-inacessible-ctor/Lib_1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package lib

import scala.deriving.Mirror

sealed trait Foo
object Foo // normally, would cache a mirror if one exists.
case class Bar private[lib] () extends Foo
case object Bar // force mirror for Bar to be anonymous.


object CallSiteSucceed {
val mFoo = summon[Mirror.SumOf[Foo]] // ok
val mBar = summon[Mirror.ProductOf[Bar]] // ok
val sampleBar = Bar()
}
6 changes: 6 additions & 0 deletions tests/run/prod-mirror-inacessible-ctor/Test_2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
@main def Test =
assert(lib.CallSiteSucceed.mFoo eq lib.Foo) // binary compatibility with 3.1
assert(lib.CallSiteSucceed.mBar ne lib.Bar) // anonymous mirror

assert(lib.CallSiteSucceed.mFoo.ordinal(lib.CallSiteSucceed.sampleBar) == 0)
assert(lib.CallSiteSucceed.mBar.fromProduct(EmptyTuple) == lib.CallSiteSucceed.sampleBar)

0 comments on commit 001bfc3

Please sign in to comment.