Skip to content

Commit b5fad12

Browse files
committed
Fix constToLiteral
We previously converted an expression with constant type to a literal if the expression was idempotent. This can hide side effects in the case where the expression is a selection from an object or lazy val. Demanding purity instead prodcues tons of errors involving inline vals on objects. We now demand idempotency if the expression refers to an inline val (or an operation over an inline val), and purity elsewhere. Fixes scala#2266
1 parent e1b9f3d commit b5fad12

File tree

6 files changed

+41
-26
lines changed

6 files changed

+41
-26
lines changed

compiler/src/dotty/tools/dotc/ast/TreeInfo.scala

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -577,21 +577,28 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] =>
577577
// blocks returning a class literal alone, even if they're idempotent.
578578
tree1
579579
case ConstantType(value) =>
580-
if (isIdempotentExpr(tree1)) Literal(value).withSpan(tree.span)
581-
else {
582-
def keepPrefix(pre: Tree) =
580+
def dropOp(t: Tree): Tree = t match
581+
case Select(pre, _) if t.tpe.isInstanceOf[ConstantType] =>
582+
// it's a primitive unary operator
583+
pre
584+
case Apply(TypeApply(Select(pre, nme.getClass_), _), Nil) =>
585+
pre
586+
case _ =>
587+
tree1
588+
589+
val countsAsPure =
590+
if dropOp(tree1).symbol.isInlineVal
591+
then isIdempotentExpr(tree1)
592+
else isPureExpr(tree1)
593+
594+
if countsAsPure then Literal(value).withSpan(tree.span)
595+
else
596+
val pre = dropOp(tree1)
597+
if pre eq tree1 then tree1
598+
else
599+
// it's a primitive unary operator or getClass call;
600+
// Simplify `pre.op` to `{ pre; v }` where `v` is the value of `pre.op`
583601
Block(pre :: Nil, Literal(value)).withSpan(tree.span)
584-
585-
tree1 match {
586-
case Select(pre, _) if tree1.tpe.isInstanceOf[ConstantType] =>
587-
// it's a primitive unary operator; Simplify `pre.op` to `{ pre; v }` where `v` is the value of `pre.op`
588-
keepPrefix(pre)
589-
case Apply(TypeApply(Select(pre, nme.getClass_), _), Nil) =>
590-
keepPrefix(pre)
591-
case _ =>
592-
tree1
593-
}
594-
}
595602
case _ => tree1
596603
}
597604
}

compiler/src/dotty/tools/dotc/transform/SymUtils.scala

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,11 @@ object SymUtils:
110110
self.isCoDefinedGiven(res.typeSymbol)
111111
self.isAllOf(Given | Method) && isCodefined(self.info)
112112

113+
// TODO Scala 3.x: only check for inline vals (no final ones)
114+
def isInlineVal(using Context) =
115+
self.isOneOf(FinalOrInline, butNot = Mutable)
116+
&& (!self.is(Method) || self.is(Accessor))
117+
113118
def useCompanionAsSumMirror(using Context): Boolean =
114119
def companionExtendsSum(using Context): Boolean =
115120
self.linkedClass.isSubClass(defn.Mirror_SumClass)

compiler/src/dotty/tools/dotc/typer/Namer.scala

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1686,11 +1686,6 @@ class Namer { typer: Typer =>
16861686
case _ =>
16871687
approxTp
16881688

1689-
// println(s"final inherited for $sym: ${inherited.toString}") !!!
1690-
// println(s"owner = ${sym.owner}, decls = ${sym.owner.info.decls.show}")
1691-
// TODO Scala 3.1: only check for inline vals (no final ones)
1692-
def isInlineVal = sym.isOneOf(FinalOrInline, butNot = Method | Mutable)
1693-
16941689
var rhsCtx = ctx.fresh.addMode(Mode.InferringReturnType)
16951690
if sym.isInlineMethod then rhsCtx = rhsCtx.addMode(Mode.InlineableBody)
16961691
if sym.is(ExtensionMethod) then rhsCtx = rhsCtx.addMode(Mode.InExtensionMethod)
@@ -1724,7 +1719,7 @@ class Namer { typer: Typer =>
17241719
// don't strip @uncheckedVariance annot for default getters
17251720
TypeOps.simplify(tp.widenTermRefExpr,
17261721
if defaultTp.exists then TypeOps.SimplifyKeepUnchecked() else null) match
1727-
case ctp: ConstantType if isInlineVal => ctp
1722+
case ctp: ConstantType if sym.isInlineVal => ctp
17281723
case tp => TypeComparer.widenInferred(tp, pt)
17291724

17301725
// Replace aliases to Unit by Unit itself. If we leave the alias in
@@ -1735,7 +1730,7 @@ class Namer { typer: Typer =>
17351730
def lhsType = fullyDefinedType(cookedRhsType, "right-hand side", mdef.span)
17361731
//if (sym.name.toString == "y") println(i"rhs = $rhsType, cooked = $cookedRhsType")
17371732
if (inherited.exists)
1738-
if (isInlineVal) lhsType else inherited
1733+
if sym.isInlineVal then lhsType else inherited
17391734
else {
17401735
if (sym.is(Implicit))
17411736
mdef match {

tests/pos/java-annot/S.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
object C {
2-
val cs: "cs" = "cs"
2+
final val cs: "cs" = "cs"
33
}
44

55
object S {

tests/run/i2266.check

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
X
2+
true
3+
1

tests/run/i2266.scala

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,13 @@
11
object Test extends App {
2-
def f = {
3-
lazy val x: true = { println("X"); true }
4-
println(x)
2+
lazy val x: true = { println("X"); true }
3+
println(x)
4+
5+
object Inner {
6+
println("Y") // not printed
7+
inline val y = 1
58
}
6-
f
9+
println(Inner.y)
10+
11+
inline val MV = Int.MaxValue
712
}
813

0 commit comments

Comments
 (0)