Skip to content

Added test-cases for initialization checker #12685

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 3 commits into from
Jun 14, 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
5 changes: 5 additions & 0 deletions tests/init/neg/Desugar.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
-- Error: tests/init/neg/Desugar.scala:12:61 ---------------------------------------------------------------------------
12 | val f: PartialFunction[A, Int] = {case C(_, rhs) => rhs.x} // error
| ^
| Calling the external method constructor $anon may cause initialization errors. Calling trace:
| -> val f: PartialFunction[A, Int] = {case C(_, rhs) => rhs.x} // error [ Desugar.scala:12 ]
15 changes: 15 additions & 0 deletions tests/init/neg/Desugar.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
trait A

class Tree[-A >: Int] {
val x: Int = 10
}

case class C[-T >: Int] (lhs: Int, rhs: Tree[T]) extends A {
val x = rhs.x
}

object DesugarError {
val f: PartialFunction[A, Int] = {case C(_, rhs) => rhs.x} // error
}


5 changes: 5 additions & 0 deletions tests/init/neg/InteractiveDriver.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
-- Error: tests/init/neg/InteractiveDriver.scala:22:71 -----------------------------------------------------------------
22 | val l2: Seq[C[?]] = l.collect{ case x: InteractiveDriver[?] => h(x) } // error
| ^
|Calling the external method constructor $anon may cause initialization errors. Calling trace:
| -> val l2: Seq[C[?]] = l.collect{ case x: InteractiveDriver[?] => h(x) } // error [ InteractiveDriver.scala:22 ]
Copy link
Contributor

Choose a reason for hiding this comment

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

I see why we have a warning here: we synthesize an anonymous local class here, but its defTree is not set. Same for the above.

23 changes: 23 additions & 0 deletions tests/init/neg/InteractiveDriver.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
trait InteractiveDriver[A <: AnyVal] {
val x: AnyVal
def g(x: A): A
}

class C[A <: AnyVal] extends InteractiveDriver[A] {
val x = 0
def g(x: A) = x
}

class D[A <: AnyVal] extends InteractiveDriver[A] {
val x = 1.5
def g(x: A) = x
}

object InteractiveDriver {
def h(x: InteractiveDriver[?]): C[?] = x match {
case c: C[?] => c
case _ => new C[Int]
}
val l: Seq[Any] = Seq(1, 2, new C[Double], new D[Int])
val l2: Seq[C[?]] = l.collect{ case x: InteractiveDriver[?] => h(x) } // error
}
8 changes: 8 additions & 0 deletions tests/init/neg/closureLeak.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
-- Error: tests/init/neg/closureLeak.scala:11:14 -----------------------------------------------------------------------
11 | l.foreach(a => a.addX(this)) // error
| ^^^^^^^^^^^^^^^^^
| Promoting the value to fully-initialized is unsafe. May only use initialized value as arguments
|
| The unsafe promotion may cause the following problem(s):
|
| 1. Promote the value under initialization to fully-initialized. May only use initialized value as arguments.
13 changes: 13 additions & 0 deletions tests/init/neg/closureLeak.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
class Outer {
val x = 10
class A(x: Int) {
var y: Int = x
def addX(o: Outer): Unit = {
y = y + o.x
}
}

val l: List[A] = List(new A(5), new A(10))
l.foreach(a => a.addX(this)) // error
val p = 10
}
8 changes: 8 additions & 0 deletions tests/init/neg/cycle-structure.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
-- Error: tests/init/neg/cycle-structure.scala:3:14 --------------------------------------------------------------------
3 | val x = B(this) // error
| ^^^^
| Promote the value under initialization to fully-initialized. May only use initialized value as arguments.
-- Error: tests/init/neg/cycle-structure.scala:9:14 --------------------------------------------------------------------
9 | val x = A(this) // error
| ^^^^
| Promote the value under initialization to fully-initialized. May only use initialized value as arguments.
11 changes: 11 additions & 0 deletions tests/init/neg/cycle-structure.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
case class A(b: B) {
val x1 = b.x
val x = B(this) // error
val y = x.a
}

case class B(a: A) {
val x1 = a.x
val x = A(this) // error
val h = x.b
}
5 changes: 5 additions & 0 deletions tests/init/neg/default-this.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
-- Error: tests/init/neg/default-this.scala:9:8 ------------------------------------------------------------------------
9 | compare() // error
| ^^^^^^^
|Promote the value under initialization to fully-initialized. May only use initialized value as arguments. Calling trace:
| -> val result = updateThenCompare(5) [ default-this.scala:11 ]
12 changes: 12 additions & 0 deletions tests/init/neg/default-this.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
class A {
var x: Int = 10
def compare(c: Int = 5, a: A = this): Boolean = if (c == a.x) true else false
}

class B extends A {
def updateThenCompare(c: Int): Boolean = {
x = c
compare() // error
}
val result = updateThenCompare(5)
}
20 changes: 20 additions & 0 deletions tests/init/neg/enum-desugared.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
-- Error: tests/init/neg/enum-desugared.scala:17:15 --------------------------------------------------------------------
17 | Array(this.LazyErrorId, this.NoExplanationID) // error // error
| ^^^^^^^^^^^^^^^^
| Promoting the value to fully-initialized is unsafe. May only use initialized value as method arguments
|
| The unsafe promotion may cause the following problem(s):
|
| 1. Calling the external method method name may cause initialization errors. Calling trace:
| -> override def productPrefix: String = this.name() [ enum-desugared.scala:29 ]
| -> Array(this.LazyErrorId, this.NoExplanationID) // error // error [ enum-desugared.scala:17 ]
-- Error: tests/init/neg/enum-desugared.scala:17:33 --------------------------------------------------------------------
17 | Array(this.LazyErrorId, this.NoExplanationID) // error // error
| ^^^^^^^^^^^^^^^^^^^^
| Promoting the value to fully-initialized is unsafe. May only use initialized value as method arguments
|
| The unsafe promotion may cause the following problem(s):
|
| 1. Calling the external method method ordinal may cause initialization errors. Calling trace:
| -> def errorNumber: Int = this.ordinal() - 2 [ enum-desugared.scala:8 ]
| -> Array(this.LazyErrorId, this.NoExplanationID) // error // error [ enum-desugared.scala:17 ]
35 changes: 35 additions & 0 deletions tests/init/neg/enum-desugared.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package example

import language.`3.0-migration`

sealed abstract class ErrorMessageID($name: String, _$ordinal: Int)
extends java.lang.Enum[ErrorMessageID]($name, _$ordinal) with scala.reflect.Enum {

def errorNumber: Int = this.ordinal() - 2
}

object ErrorMessageID {

final val LazyErrorId = $new(0, "LazyErrorId")
final val NoExplanationID = $new(1, "NoExplanationID")

private[this] val $values: Array[ErrorMessageID] =
Array(this.LazyErrorId, this.NoExplanationID) // error // error

def values: Array[ErrorMessageID] = $values.clone()

def valueOf($name: String): ErrorMessageID = $name match {
case "LazyErrorId" => this.LazyErrorId
case "NoExplanationID" => this.NoExplanationID
case _ => throw new IllegalArgumentException("enum case not found: " + $name)
}

private[this] def $new(_$ordinal: Int, $name: String): ErrorMessageID =
new ErrorMessageID($name, _$ordinal) with scala.runtime.EnumValue {
override def productPrefix: String = this.name()
}

def fromOrdinal(ordinal: Int): ErrorMessageID =
try ErrorMessageID.$values.apply(ordinal)
catch { case _ => throw new NoSuchElementException(ordinal.toString()) }
}
9 changes: 9 additions & 0 deletions tests/init/neg/enum.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
-- Error: tests/init/neg/enum.scala:4:8 --------------------------------------------------------------------------------
4 | NoExplanationID // error
| ^
| Promoting the value to fully-initialized is unsafe. May only use initialized value as method arguments
|
| The unsafe promotion may cause the following problem(s):
|
| 1. Calling the external method method name may cause initialization errors. Calling trace:
| -> NoExplanationID // error [ enum.scala:4 ]
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can add another example for the warning. You can compile the file as follows in sbt:

sbt > scalac -d out -Xprint:initChecker -Ysafe-init tests/init/neg/enum.scala

With desugared code, it's easier to see what's happening.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The desugared code from Xprint contains many annotations and cannot pass the compilation. What modifications do I need?

Copy link
Contributor

Choose a reason for hiding this comment

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

The from -Xprint is meant to help you create examples with the same warning but without using enums.

6 changes: 6 additions & 0 deletions tests/init/neg/enum.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
enum ErrorMessageID extends java.lang.Enum[ErrorMessageID] {
case
LazyErrorId,
NoExplanationID // error
def errorNumber = ordinal - 2
}
8 changes: 8 additions & 0 deletions tests/init/neg/leak-warm.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
-- Error: tests/init/neg/leak-warm.scala:18:26 -------------------------------------------------------------------------
18 | val l: List[A] = List(c, d) // error // error
| ^
|Promote the value under initialization to fully-initialized. May only use initialized value as method arguments.
-- Error: tests/init/neg/leak-warm.scala:18:29 -------------------------------------------------------------------------
18 | val l: List[A] = List(c, d) // error // error
| ^
|Promote the value under initialization to fully-initialized. May only use initialized value as method arguments.
20 changes: 20 additions & 0 deletions tests/init/neg/leak-warm.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
object leakWarm {
abstract class A(tag: Int) {
class B(x: Int) {
val y = x
}
def m(): B
}

class C(tag1: Int, tag2: Int) extends A(tag1) {
def m() = new B(5)
}

class D(tag1: Int, tag2: Int) extends A(tag1 + tag2) {
def m() = new B(tag1)
}
val c = new C(1, 2)
val d = new D(3, 4)
val l: List[A] = List(c, d) // error // error
val l2 = l.map(_.m())
Copy link
Contributor

Choose a reason for hiding this comment

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

This file needs to be updated because of merge conflicts.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've fixed that

}
5 changes: 5 additions & 0 deletions tests/init/neg/local-warm.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
-- Error: tests/init/neg/local-warm.scala:12:12 ------------------------------------------------------------------------
12 | val t = m1() // error
| ^^^^^^^^^^^^
|Promote the value under initialization to fully-initialized. Local definitions may only hold initialized values. Calling trace:
| -> val x = g() [ local-warm.scala:15 ]
16 changes: 16 additions & 0 deletions tests/init/neg/local-warm.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
abstract class A {
def m() = 10
def m1(): B = new B
def m2(): Int = m1().m()
class B extends A {
def x = 10
}
}

class C extends A {
def g() = {
val t = m1() // error
Copy link
Contributor

Choose a reason for hiding this comment

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

Opportunity for improvement: for local definitions (not params), we can re-compute its value from its rhs. There is no need to require it be hot.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i've fixed that

t.x
}
val x = g()
}
8 changes: 8 additions & 0 deletions tests/init/pos/leak-this-inner.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
class A {
val x = 10
class B(a: A) {
val anotherX = A.this.x
}
val b = B(this) // error
val xAgain = b.anotherX
}
11 changes: 11 additions & 0 deletions tests/init/pos/leak-this.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
class Parent {
val child: Child = new Child(this) // error
}

class Child(parent: Parent) {
val friend = new Friend(this.parent)
}

class Friend(parent: Parent) {
val tag = 10
}
11 changes: 11 additions & 0 deletions tests/init/pos/methodAtLast.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
class Foo(x: Int) {
var y: Int = x
case class Bar(z: Int) extends Foo(z)
def updateY(n: Int): Unit = {
if (y < 20) {
val b = new Bar(x + n)
y = b.z
}
}
updateY(5)
}