From 8daf2b730c468906c78221e9de31ff2e4805bfd5 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 26 Apr 2021 11:18:55 +0200 Subject: [PATCH 1/2] Avoid creating cyclic types when reporting errors in selectionType The previous code was a great demonstration of the perils of mixing mutable variables with delayed evaluation. Also, make lazy vals in Message @threadUnsafe. This means that any recursion (like the one we encountered before the fix) manifests itself in stackoverflows instead of deadlocks. Fixes #12220 --- .../src/dotty/tools/dotc/reporting/Message.scala | 6 +++--- .../src/dotty/tools/dotc/typer/TypeAssigner.scala | 12 +++++++----- tests/neg/i12220.scala | 3 +++ 3 files changed, 13 insertions(+), 8 deletions(-) create mode 100644 tests/neg/i12220.scala diff --git a/compiler/src/dotty/tools/dotc/reporting/Message.scala b/compiler/src/dotty/tools/dotc/reporting/Message.scala index bed1d955c1a4..43eedaafdc87 100644 --- a/compiler/src/dotty/tools/dotc/reporting/Message.scala +++ b/compiler/src/dotty/tools/dotc/reporting/Message.scala @@ -1,7 +1,7 @@ package dotty.tools package dotc package reporting - +import scala.annotation.threadUnsafe import util.SourcePosition object Message { @@ -85,10 +85,10 @@ abstract class Message(val errorId: ErrorMessageID) { self => def rawMessage = message /** The message to report. tags are filtered out */ - lazy val message: String = dropNonSensical(msg + msgSuffix) + @threadUnsafe lazy val message: String = dropNonSensical(msg + msgSuffix) /** The explanation to report. tags are filtered out */ - lazy val explanation: String = dropNonSensical(explain) + @threadUnsafe lazy val explanation: String = dropNonSensical(explain) /** A message is non-sensical if it contains references to * tags. Such tags are inserted by the error diagnostic framework if a diff --git a/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala b/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala index 8ce74a6061cc..82704e262cd4 100644 --- a/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala +++ b/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala @@ -118,11 +118,13 @@ trait TypeAssigner { /** The type of the selection `tree`, where `qual1` is the typed qualifier part. */ def selectionType(tree: untpd.RefTree, qual1: Tree)(using Context): Type = var qualType = qual1.tpe.widenIfUnstable - if !qualType.hasSimpleKind && tree.name != nme.CONSTRUCTOR then - // constructors are selected on typeconstructor, type arguments are passed afterwards - qualType = errorType(em"$qualType takes type parameters", qual1.srcPos) - else if !qualType.isInstanceOf[TermType] then - qualType = errorType(em"$qualType is illegal as a selection prefix", qual1.srcPos) + if !qualType.isError then + val prevQual = qualType + if !qualType.hasSimpleKind && tree.name != nme.CONSTRUCTOR then + // constructors are selected on typeconstructor, type arguments are passed afterwards + qualType = errorType(em"$prevQual takes type parameters", qual1.srcPos) + else if !qualType.isInstanceOf[TermType] then + qualType = errorType(em"$prevQual is illegal as a selection prefix", qual1.srcPos) def arrayElemType = qual1.tpe.widen match case JavaArrayType(elemtp) => elemtp diff --git a/tests/neg/i12220.scala b/tests/neg/i12220.scala new file mode 100644 index 000000000000..c1380b8781b5 --- /dev/null +++ b/tests/neg/i12220.scala @@ -0,0 +1,3 @@ +val a: List[Any] = List(List(1,2), List(3,4)) +val _ = for(b <- a ; c <- b.asInstanceOf[List]) { println(c) } // error + From c62db949857566dd1de42cf56fa6891dc579a08e Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 26 Apr 2021 11:49:52 +0200 Subject: [PATCH 2/2] Avoid creating a variable in the first place --- .../src/dotty/tools/dotc/typer/TypeAssigner.scala | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala b/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala index 82704e262cd4..2d6311f76ebe 100644 --- a/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala +++ b/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala @@ -117,14 +117,15 @@ trait TypeAssigner { /** The type of the selection `tree`, where `qual1` is the typed qualifier part. */ def selectionType(tree: untpd.RefTree, qual1: Tree)(using Context): Type = - var qualType = qual1.tpe.widenIfUnstable - if !qualType.isError then - val prevQual = qualType - if !qualType.hasSimpleKind && tree.name != nme.CONSTRUCTOR then + val qualType0 = qual1.tpe.widenIfUnstable + val qualType = + if !qualType0.hasSimpleKind && tree.name != nme.CONSTRUCTOR then // constructors are selected on typeconstructor, type arguments are passed afterwards - qualType = errorType(em"$prevQual takes type parameters", qual1.srcPos) - else if !qualType.isInstanceOf[TermType] then - qualType = errorType(em"$prevQual is illegal as a selection prefix", qual1.srcPos) + errorType(em"$qualType0 takes type parameters", qual1.srcPos) + else if !qualType0.isInstanceOf[TermType] && !qualType0.isError then + errorType(em"$qualType0 is illegal as a selection prefix", qual1.srcPos) + else + qualType0 def arrayElemType = qual1.tpe.widen match case JavaArrayType(elemtp) => elemtp