Skip to content
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

Search for type tests when matching abstract applied type #13453

Merged
merged 1 commit into from
Oct 8, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/typer/Synthesizer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class Synthesizer(typer: Typer)(using @constructorOnly c: Context):
(formal, span) => formal.argInfos match {
case arg1 :: arg2 :: Nil if !defn.isBottomClass(arg2.typeSymbol) =>
val tp1 = fullyDefinedType(arg1, "TypeTest argument", span)
val tp2 = fullyDefinedType(arg2, "TypeTest argument", span)
val tp2 = fullyDefinedType(arg2, "TypeTest argument", span).normalized
Copy link
Contributor

Choose a reason for hiding this comment

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

That looks a bit suspicious. By default I would say that we need a really good reason to manually trigger type normalized. In principle all newly created types should already go thought nurmalization just after creation, and if fullyDefinedType creates non normalized types it might mean that there is a better spot in the code to do the normalization (deeper in the stack trace).

Note that we another place in the code where we do a fullyDefinedType call followed by a .normalized (which is equality suspicious):

https://github.com/lampepfl/dotty/blob/fe24fd3af2d985262b67394ac8d606a9830e0291/compiler/src/dotty/tools/dotc/typer/Synthesizer.scala#L163

My intition tells me that there is a way to both fix this issue and remove that .normalized in Synthesizer, and that this way would a better solution that the change done in this PR...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Then it seems to be a general limitation. We should probably open a separate issue for this improvement.

val sym2 = tp2.typeSymbol
if tp1 <:< tp2 then
// optimization when we know the typetest will always succeed
Expand Down
42 changes: 23 additions & 19 deletions compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -901,28 +901,32 @@ class Typer extends Namer
* exists, rewrite to `tt(e)`.
* @pre We are in pattern-matching mode (Mode.Pattern)
*/
def tryWithTypeTest(tree: Typed, pt: Type)(using Context): Tree = tree.tpt.tpe.dealias match {
case tref: TypeRef if !tref.symbol.isClass && !ctx.isAfterTyper && !(tref =:= pt) =>
def withTag(tpe: Type): Option[Tree] = {
require(ctx.mode.is(Mode.Pattern))
withoutMode(Mode.Pattern)(
inferImplicit(tpe, EmptyTree, tree.tpt.span)
) match
case SearchSuccess(clsTag, _, _, _) =>
withMode(Mode.InTypeTest) {
Some(typed(untpd.Apply(untpd.TypedSplice(clsTag), untpd.TypedSplice(tree.expr)), pt))
}
case _ =>
None
}
val tag = withTag(defn.TypeTestClass.typeRef.appliedTo(pt, tref))
.orElse(withTag(defn.ClassTagClass.typeRef.appliedTo(tref)))
def tryWithTypeTest(tree: Typed, pt: Type)(using Context): Tree =
def withTag(tpe: Type): Option[Tree] = {
require(ctx.mode.is(Mode.Pattern))
withoutMode(Mode.Pattern)(
inferImplicit(tpe, EmptyTree, tree.tpt.span)
) match
case SearchSuccess(clsTag, _, _, _) =>
withMode(Mode.InTypeTest) {
Some(typed(untpd.Apply(untpd.TypedSplice(clsTag), untpd.TypedSplice(tree.expr)), pt))
}
case _ =>
None
}
def tagged(tpe: Type) = {
val tag = withTag(defn.TypeTestClass.typeRef.appliedTo(pt, tpe))
.orElse(withTag(defn.ClassTagClass.typeRef.appliedTo(tpe)))
.getOrElse(tree)
if tag.symbol.owner == defn.ClassTagClass && config.Feature.sourceVersion.isAtLeast(config.SourceVersion.future) then
if tag.symbol.maybeOwner == defn.ClassTagClass && config.Feature.sourceVersion.isAtLeast(config.SourceVersion.future) then
report.warning("Use of `scala.reflect.ClassTag` for type testing may be unsound. Consider using `scala.reflect.TypeTest` instead.", tree.srcPos)
tag
case _ => tree
}
}
tree.tpt.tpe.dealias match {
case tpe @ AppliedType(tref: TypeRef, _) if !tref.symbol.isClass && !ctx.isAfterTyper && !(tpe =:= pt) => tagged(tpe)
case tref: TypeRef if !tref.symbol.isClass && !ctx.isAfterTyper && !(tref =:= pt) => tagged(tref)
case _ => tree
}


def typedNamedArg(tree: untpd.NamedArg, pt: Type)(using Context): NamedArg = {
Expand Down
2 changes: 2 additions & 0 deletions compiler/test/dotc/run-test-pickling.blacklist
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ i7212
i7868.scala
i9011.scala
i9473.scala
i13433.scala
i13433b.scala
macros-in-same-project1
mixin-forwarder-overload
t10889
Expand Down
32 changes: 32 additions & 0 deletions tests/pos-special/fatal-warnings/i13433.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import scala.reflect.TypeTest

type Matcher[A] = A match { case String => String }

def patternMatch[A](a: Any)(using tt: TypeTest[Any, Matcher[A]]): Option[Matcher[A]] = {
// type T = RDF.Triple[Rdf]
a match {
case res: Matcher[A] => Some(res)
case _ => None
}
}

def patternMatchWithAlias[A](a: Any)(using tt: TypeTest[Any, Matcher[A]]): Option[Matcher[A]] = {
type T = Matcher[A]
a match {
case res: T => Some(res)
case _ => None
}
}


@main def main = {
println(patternMatch[String]("abc"))
println(patternMatchWithAlias[String]("abc"))
println(patternMatch[String]("abc")(using (s: Any) => {
if s.isInstanceOf[Matcher[String]] then Some[s.type & Matcher[String]](s.asInstanceOf[s.type & Matcher[String]]) else None }))
println(patternMatchWithAlias[String]("abc")(using (s: Any) => {
if s.isInstanceOf[Matcher[String]] then Some[s.type & Matcher[String]](s.asInstanceOf[s.type & Matcher[String]]) else None }))

println(patternMatch[String](1))
println(patternMatchWithAlias[String](1))
}
28 changes: 28 additions & 0 deletions tests/pos-special/fatal-warnings/i13433b.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import scala.reflect.ClassTag

type Matcher[A] = A match { case String => String }

def patternMatch[A](a: Any)(using tt: ClassTag[Matcher[A]]): Option[Matcher[A]] = {
// type T = RDF.Triple[Rdf]
a match {
case res: Matcher[A] => Some(res)
case _ => None
}
}

def patternMatchWithAlias[A](a: Any)(using tt: ClassTag[Matcher[A]]): Option[Matcher[A]] = {
type T = Matcher[A]
a match {
case res: T => Some(res)
case _ => None
}
}


@main def main = {
println(patternMatch[String]("abc"))
println(patternMatchWithAlias[String]("abc"))

println(patternMatch[String](1))
println(patternMatchWithAlias[String](1))
}
4 changes: 4 additions & 0 deletions tests/run/i13433.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Some(abc)
Some(abc)
None
None
28 changes: 28 additions & 0 deletions tests/run/i13433.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import scala.reflect.TypeTest

type Matcher[A] = A match { case String => String }

def patternMatch[A](a: Any)(using tt: TypeTest[Any, Matcher[A]]): Option[Matcher[A]] = {
// type T = RDF.Triple[Rdf]
a match {
case res: Matcher[A] => Some(res)
case _ => None
}
}

def patternMatchWithAlias[A](a: Any)(using tt: TypeTest[Any, Matcher[A]]): Option[Matcher[A]] = {
type T = Matcher[A]
a match {
case res: T => Some(res)
case _ => None
}
}


@main def Test = {
println(patternMatch[String]("abc"))
println(patternMatchWithAlias[String]("abc"))

println(patternMatch[String](1))
println(patternMatchWithAlias[String](1))
}
4 changes: 4 additions & 0 deletions tests/run/i13433b.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Some(abc)
Some(abc)
None
None
28 changes: 28 additions & 0 deletions tests/run/i13433b.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import scala.reflect.ClassTag

type Matcher[A] = A match { case String => String }

def patternMatch[A](a: Any)(using tt: ClassTag[Matcher[A]]): Option[Matcher[A]] = {
// type T = RDF.Triple[Rdf]
a match {
case res: Matcher[A] => Some(res)
case _ => None
}
}

def patternMatchWithAlias[A](a: Any)(using tt: ClassTag[Matcher[A]]): Option[Matcher[A]] = {
type T = Matcher[A]
a match {
case res: T => Some(res)
case _ => None
}
}


@main def Test = {
println(patternMatch[String]("abc"))
println(patternMatchWithAlias[String]("abc"))

println(patternMatch[String](1))
println(patternMatchWithAlias[String](1))
}