Skip to content

Fix type test for trait parameter arguments #12041

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions compiler/src/dotty/tools/dotc/ast/TreeInfo.scala
Original file line number Diff line number Diff line change
Expand Up @@ -652,7 +652,7 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] =>
}
}

/** The type arguemnts of a possibly curried call */
/** The type arguments of a possibly curried call */
def typeArgss(tree: Tree): List[List[Tree]] =
@tailrec
def loop(tree: Tree, argss: List[List[Tree]]): List[List[Tree]] = tree match
Expand All @@ -661,7 +661,7 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] =>
case _ => argss
loop(tree, Nil)

/** The term arguemnts of a possibly curried call */
/** The term arguments of a possibly curried call */
def termArgss(tree: Tree): List[List[Tree]] =
@tailrec
def loop(tree: Tree, argss: List[List[Tree]]): List[List[Tree]] = tree match
Expand All @@ -670,7 +670,7 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] =>
case _ => argss
loop(tree, Nil)

/** The type and term arguemnts of a possibly curried call, in the order they are given */
/** The type and term arguments of a possibly curried call, in the order they are given */
def allArgss(tree: Tree): List[List[Tree]] =
@tailrec
def loop(tree: Tree, argss: List[List[Tree]]): List[List[Tree]] = tree match
Expand Down
4 changes: 4 additions & 0 deletions compiler/src/dotty/tools/dotc/core/SymDenotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2199,6 +2199,10 @@ object SymDenotations {
def paramAccessors(using Context): List[Symbol] =
unforcedDecls.filter(_.is(ParamAccessor))

/** The term parameter getters of this class. */
def paramGetters(using Context): List[Symbol] =
paramAccessors.filterNot(_.isSetter)

/** If this class has the same `decls` scope reference in `phase` and
* `phase.next`, install a new denotation with a cloned scope in `phase.next`.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ enum ErrorMessageID extends java.lang.Enum[ErrorMessageID] {
ErasedTypesCanOnlyBeFunctionTypesID,
CaseClassMissingNonImplicitParamListID,
EnumerationsShouldNotBeEmptyID,
UNUSED_1,
IllegalParameterInitID,
RedundantModifierID,
TypedCaseDoesNotExplicitlyExtendTypedEnumID,
IllegalRedefinitionOfStandardKindID,
Expand Down
8 changes: 8 additions & 0 deletions compiler/src/dotty/tools/dotc/reporting/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1467,6 +1467,14 @@ import transform.SymUtils._
def msg = em"""$tp does not conform to its self type $selfType; cannot be instantiated"""
}

class IllegalParameterInit(found: Type, expected: Type, param: Symbol, cls: Symbol)(using Context)
extends TypeMismatchMsg(found, expected)(IllegalParameterInitID):
def msg =
em"""illegal parameter initialization of $param.
|
| The argument passed for $param has type: $found
| but $cls expects $param to have type: $expected"""

class AbstractMemberMayNotHaveModifier(sym: Symbol, flag: FlagSet)(
implicit ctx: Context)
extends SyntaxMsg(AbstractMemberMayNotHaveModifierID) {
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/transform/Constructors.scala
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ class Constructors extends MiniPhase with IdentityDenotTransformer { thisPhase =
// Produce aligned accessors and constructor parameters. We have to adjust
// for any outer parameters, which are last in the sequence of original
// parameter accessors but come first in the constructor parameter list.
val accessors = cls.paramAccessors.filterNot(x => x.isSetter)
val accessors = cls.paramGetters
val vparamsWithOuterLast = vparams match {
case vparam :: rest if vparam.name == nme.OUTER => rest ::: vparam :: Nil
case _ => vparams
Expand Down
50 changes: 22 additions & 28 deletions compiler/src/dotty/tools/dotc/typer/RefChecks.scala
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import Constants.Constant
import NullOpsDecorator._

object RefChecks {
import tpd.{Tree, MemberDef, NamedArg, Literal, Template, DefDef}
import tpd._

val name: String = "refchecks"

Expand Down Expand Up @@ -133,6 +133,27 @@ object RefChecks {
false
}

/** Check that arguments passed to trait parameters conform to the parameter types
* in the current class. This is necessary since parameter types might be narrowed
* through intersection with other parent traits. See neg/i11018.scala.
*/
def checkParamInits(app: Apply): Unit =
val parentCls = app.tpe.classSymbol
if parentCls.is(Trait) then
val params = parentCls.asClass.paramGetters
val args = termArgss(app).flatten
for (param, arg) <- params.lazyZip(args) do
if !param.is(Private) then // its type can be narrowed through intersection -> a check is needed
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a good idea to check this one here, I didn't do that in #11059.

val paramType = cls.thisType.memberInfo(param)
if !(arg.tpe <:< paramType) then
val argTypes = args.tpes
// it could still be OK but we might need to substitute arguments for parameters
// to account for dependent parameters. See pos/i11993.scala
if !(arg.tpe.subst(params, argTypes) <:< paramType.subst(params, argTypes))
then
report.error(IllegalParameterInit(arg.tpe, paramType, param, cls), arg.srcPos)

for case app: Apply <- parentTrees do checkParamInits(app)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, it's also a better idea to walk through the constructors.

case _ =>
}

Expand Down Expand Up @@ -801,34 +822,7 @@ object RefChecks {
report.error(problem(), clazz.srcPos)
}

// check that basetype and subtype agree on types of trait parameters
//
// I.e. trait and class parameters not only need to conform to the expected
// type of the corresponding base-trait, but also to the type as seen by the
// inheriting subtype.
def checkTraitParametersOK() = for {
parent <- clazz.info.parents
parentSym = parent.classSymbol
if parentSym.isClass
cls = parentSym.asClass
if cls.paramAccessors.nonEmpty
param <- cls.paramAccessors
} {
val tpeFromParent = parent.memberInfo(param)
val tpeFromClazz = clazz.thisType.memberInfo(param)
if (!(tpeFromParent <:< tpeFromClazz)) {
val msg =
em"""illegal parameter: The types of $param do not match.
|
| $param in $cls has type: $tpeFromParent
| but $clazz expects $param to have type: $tpeFromClazz"""

report.error(msg, clazz.srcPos)
}
}

checkParameterizedTraitsOK()
checkTraitParametersOK()
}

/** Check that `site` does not inherit conflicting generic instances of `baseCls`,
Expand Down
2 changes: 1 addition & 1 deletion tests/generic-java-signatures/derivedNames.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ object Test {
if (scala.util.Properties.isWin)
assert(returnType.toString == out1 || returnType.toString == out2)
else if (scala.util.Properties.isMac)
assert(returnType.toString == out1)
assert(returnType.toString == out1, s"$returnType != $out1")
else
assert(returnType.toString == out2)
}
Expand Down
2 changes: 1 addition & 1 deletion tests/neg/i3989f.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ object Test extends App {
class B[+X](val y: X) extends A[X](y)
class C extends B(5) with A[String] // error: illegal inheritance

class D extends B(5) with A[Any] // error: illegal parameter
class D extends B(5) with A[Any]

def f(a: A[Int]): String = a match {
case c: C => c.x
Expand Down
15 changes: 15 additions & 0 deletions tests/pos/i11993.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
object test1:
class Foo(
val x: String,
val y: Option[x.type]
)

class Bar extends Foo("bar", Some("bar"))

object test2:
trait Foo(
val x: String,
val y: Option[x.type]
)

class Bar extends Foo("bar", Some("bar"))