Skip to content

Commit

Permalink
Don't compute least upper bounds for expressions in statement positio…
Browse files Browse the repository at this point in the history
…ns inside blocks.

This may save huge amount of time (Fixes SI-5862) for complicated lubs. I had to remove the a check in adapt for
the part that transforms <expr> into { <expr>; () } when the expected type is Unit. The reason is in the
code.

As a side effect, we get more warnings for pure expressions in statement positions (see the change in the test file).
  • Loading branch information
dragos committed Jun 3, 2012
1 parent 71006c0 commit 037d3dc
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 9 deletions.
14 changes: 13 additions & 1 deletion src/compiler/scala/tools/nsc/typechecker/Modes.scala
Expand Up @@ -86,6 +86,17 @@ trait Modes {
*/
final val TYPEPATmode = 0x10000

/** STATmode is set when typing statements inside a block.
*
* This is useful only for skipping lub computations in
* such positions, when the expected type is known to be Unit. This
* saves time in pathological cases where lubs can take many seconds
* but in the end is discarded.
*
* TODO: Remove when lubs become types in their own right.
*/
final val STATmode = 0x20000

final private val StickyModes = EXPRmode | PATTERNmode | TYPEmode | ALTmode

final def onlyStickyModes(mode: Int) =
Expand Down Expand Up @@ -128,7 +139,8 @@ trait Modes {
(1 << 13) -> "ALTmode",
(1 << 14) -> "HKmode",
(1 << 15) -> "BYVALmode",
(1 << 16) -> "TYPEPATmode"
(1 << 16) -> "TYPEPATmode",
(1 << 17) -> "STATmode"
)
def modeString(mode: Int): String =
if (mode == 0) "NOmode"
Expand Down
13 changes: 8 additions & 5 deletions src/compiler/scala/tools/nsc/typechecker/Typers.scala
Expand Up @@ -1127,9 +1127,11 @@ trait Typers extends Modes with Adaptations with Taggings {
if (inExprModeButNot(mode, FUNmode)) {
pt.normalize match {
case TypeRef(_, sym, _) =>
// note: was if (pt.typeSymbol == UnitClass) but this leads to a potentially
// infinite expansion if pt is constant type ()
if (sym == UnitClass && tree.tpe <:< AnyClass.tpe) { // (12)
// note: there was an additional '&& tree.tpe <:< AnyClass.tpe', claiming that
// it would save us from a potentially infinite expansion if pt is constant type ()
// didn't seem to do any good, and the test would miss the fact that Object.tpe <:< AnyClass.tpe
// after erasure (triggered by try-catches added by cleanup for structural types)
if (sym == UnitClass) { // (12)
if (settings.warnValueDiscard.value)
context.unit.warning(tree.pos, "discarded non-Unit value")
return typed(atPos(tree.pos)(Block(List(tree), Literal(Constant()))), mode, pt)
Expand Down Expand Up @@ -2566,7 +2568,7 @@ trait Typers extends Modes with Adaptations with Taggings {
} else newTyper(context.make(stat, exprOwner))
// XXX this creates a spurious dead code warning if an exception is thrown
// in a constructor, even if it is the only thing in the constructor.
val result = checkDead(localTyper.typed(stat, EXPRmode | BYVALmode, WildcardType))
val result = checkDead(localTyper.typed(stat, EXPRmode | BYVALmode | STATmode, WildcardType))

if (treeInfo.isSelfOrSuperConstrCall(result)) {
context.inConstructorSuffix = true
Expand Down Expand Up @@ -3861,6 +3863,7 @@ trait Typers extends Modes with Adaptations with Taggings {
&& thenTp =:= elseTp
) (thenp1.tpe, false) // use unpacked type
// TODO: skolemize (lub of packed types) when that no longer crashes on files/pos/t4070b.scala
else if ((mode & STATmode) != 0) (definitions.UnitClass.tpe, true)
else ptOrLub(List(thenp1.tpe, elsep1.tpe), pt)

if (needAdapt) { //isNumericValueType(owntype)) {
Expand Down Expand Up @@ -4744,7 +4747,7 @@ trait Typers extends Modes with Adaptations with Taggings {
var catches1 = typedCases(catches, ThrowableClass.tpe, pt)
val finalizer1 = if (finalizer.isEmpty) finalizer
else typed(finalizer, UnitClass.tpe)
val (owntype, needAdapt) = ptOrLub(block1.tpe :: (catches1 map (_.tpe)), pt)
val (owntype, needAdapt) = if ((mode & STATmode) != 0) (definitions.UnitClass.tpe, true) else ptOrLub(block1.tpe :: (catches1 map (_.tpe)), pt)
if (needAdapt) {
block1 = adapt(block1, mode, owntype)
catches1 = catches1 map (adaptCase(_, mode, owntype))
Expand Down
4 changes: 2 additions & 2 deletions test/files/neg/checksensible.check
Expand Up @@ -77,8 +77,8 @@ checksensible.scala:63: error: comparing a fresh object using `!=' will always y
new Exception() != new Exception()
^
checksensible.scala:66: error: comparing values of types Int and Null using `==' will always yield false
if (foo.length == null) "plante" else "plante pas"
^
val dummy = if (foo.length == null) "plante" else "plante pas"
^
checksensible.scala:71: error: comparing values of types Bip and Bop using `==' will always yield false
(x1 == x2)
^
Expand Down
2 changes: 1 addition & 1 deletion test/files/neg/checksensible.scala
Expand Up @@ -63,7 +63,7 @@ class EqEqRefTest {
new Exception() != new Exception()

val foo: Array[String] = Array("1","2","3")
if (foo.length == null) "plante" else "plante pas"
val dummy = if (foo.length == null) "plante" else "plante pas"

// final classes with default equals
val x1 = new Bip
Expand Down

0 comments on commit 037d3dc

Please sign in to comment.