-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add supports for type cast and filtering type for field and method owner in global initialization checker #19612
Changes from 7 commits
16ecedd
f091c4e
124f68b
113002b
8412f78
0a95653
36392fa
229062c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,6 +27,7 @@ import scala.collection.immutable.ListSet | |
import scala.collection.mutable | ||
import scala.annotation.tailrec | ||
import scala.annotation.constructorOnly | ||
import dotty.tools.dotc.core.Flags.AbstractOrTrait | ||
|
||
/** Check initialization safety of static objects | ||
* | ||
|
@@ -203,6 +204,7 @@ object Objects: | |
|
||
/** | ||
* Represents a lambda expression | ||
* @param klass The enclosing class of the anonymous function's creation site | ||
*/ | ||
case class Fun(code: Tree, thisV: ThisValue, klass: ClassSymbol, env: Env.Data) extends ValueElement: | ||
def show(using Context) = "Fun(" + code.show + ", " + thisV.show + ", " + klass.show + ")" | ||
|
@@ -599,6 +601,26 @@ object Objects: | |
|
||
case _ => a | ||
|
||
def filterType(tpe: Type)(using Context): Value = | ||
val baseClasses = tpe.baseClasses | ||
if baseClasses.isEmpty then a | ||
else tpe match | ||
case t @ SAMType(_, _) if a.isInstanceOf[Fun] => a // if tpe is SAMType and a is Fun, allow it | ||
case _ => filterClass(baseClasses.head) // could have called ClassSymbol, but it does not handle OrType and AndType | ||
|
||
def filterClass(sym: Symbol)(using Context): Value = | ||
if !sym.isClass then a | ||
else | ||
val klass = sym.asClass | ||
a match | ||
case Cold => Cold | ||
case ref: Ref if ref.klass.isSubClass(klass) => ref | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For consistency with other cases, I'd move the |
||
case ref: Ref => Bottom | ||
case ValueSet(values) => values.map(v => v.filterClass(klass)).join | ||
case arr: OfArray => if defn.ArrayClass.isSubClass(klass) then arr else Bottom | ||
case fun: Fun => | ||
if klass.isOneOf(AbstractOrTrait) && klass.baseClasses.exists(defn.isFunctionClass) then fun else Bottom | ||
|
||
extension (value: Ref | Cold.type) | ||
def widenRefOrCold(height : Int)(using Context) : Ref | Cold.type = value.widen(height).asInstanceOf[ThisValue] | ||
|
||
|
@@ -617,7 +639,7 @@ object Objects: | |
* @param needResolve Whether the target of the call needs resolution? | ||
*/ | ||
def call(value: Value, meth: Symbol, args: List[ArgInfo], receiver: Type, superType: Type, needResolve: Boolean = true): Contextual[Value] = log("call " + meth.show + ", this = " + value.show + ", args = " + args.map(_.value.show), printer, (_: Value).show) { | ||
value match | ||
value.filterClass(meth.owner) match | ||
case Cold => | ||
report.warning("Using cold alias. " + Trace.show, Trace.position) | ||
Bottom | ||
|
@@ -733,7 +755,6 @@ object Objects: | |
* @param args Arguments of the constructor call (all parameter blocks flatten to a list). | ||
*/ | ||
def callConstructor(value: Value, ctor: Symbol, args: List[ArgInfo]): Contextual[Value] = log("call " + ctor.show + ", args = " + args.map(_.value.show), printer, (_: Value).show) { | ||
|
||
value match | ||
case ref: Ref => | ||
if ctor.hasSource then | ||
|
@@ -768,7 +789,7 @@ object Objects: | |
* @param needResolve Whether the target of the selection needs resolution? | ||
*/ | ||
def select(value: Value, field: Symbol, receiver: Type, needResolve: Boolean = true): Contextual[Value] = log("select " + field.show + ", this = " + value.show, printer, (_: Value).show) { | ||
value match | ||
value.filterClass(field.owner) match | ||
case Cold => | ||
report.warning("Using cold alias", Trace.position) | ||
Bottom | ||
|
@@ -839,12 +860,12 @@ object Objects: | |
* @param rhsTyp The type of the right-hand side. | ||
*/ | ||
def assign(lhs: Value, field: Symbol, rhs: Value, rhsTyp: Type): Contextual[Value] = log("Assign" + field.show + " of " + lhs.show + ", rhs = " + rhs.show, printer, (_: Value).show) { | ||
lhs match | ||
lhs.filterClass(field.owner) match | ||
case fun: Fun => | ||
report.warning("[Internal error] unexpected tree in assignment, fun = " + fun.code.show + Trace.show, Trace.position) | ||
|
||
case arr: OfArray => | ||
report.warning("[Internal error] unexpected tree in assignment, array = " + arr.show + Trace.show, Trace.position) | ||
report.warning("[Internal error] unexpected tree in assignment, array = " + arr.show + " field = " + field + Trace.show, Trace.position) | ||
|
||
case Cold => | ||
report.warning("Assigning to cold aliases is forbidden. " + Trace.show, Trace.position) | ||
|
@@ -876,8 +897,7 @@ object Objects: | |
* @param args The arguments passsed to the constructor. | ||
*/ | ||
def instantiate(outer: Value, klass: ClassSymbol, ctor: Symbol, args: List[ArgInfo]): Contextual[Value] = log("instantiating " + klass.show + ", outer = " + outer + ", args = " + args.map(_.value.show), printer, (_: Value).show) { | ||
outer match | ||
|
||
outer.filterClass(klass.owner) match | ||
case _ : Fun | _: OfArray => | ||
report.warning("[Internal error] unexpected outer in instantiating a class, outer = " + outer.show + ", class = " + klass.show + ", " + Trace.show, Trace.position) | ||
Bottom | ||
|
@@ -1091,6 +1111,9 @@ object Objects: | |
instantiate(outer, cls, ctor, args) | ||
} | ||
|
||
case TypeCast(elem, tpe) => | ||
eval(elem, thisV, klass).filterType(tpe) | ||
|
||
case Apply(ref, arg :: Nil) if ref.symbol == defn.InitRegionMethod => | ||
val regions2 = Regions.extend(expr.sourcePos) | ||
if Regions.exists(expr.sourcePos) then | ||
|
@@ -1549,7 +1572,7 @@ object Objects: | |
report.warning("The argument should be a constant integer value", arg) | ||
res.widen(1) | ||
case _ => | ||
res.widen(1) | ||
if res.isInstanceOf[Fun] then res.widen(2) else res.widen(1) // TODO: changing to widen(2) causes standard library analysis to loop infinitely | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove the comment because we now know it's not infinite. |
||
|
||
argInfos += ArgInfo(widened, trace.add(arg.tree), arg.tree) | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
object A { | ||
val f: Int = 10 | ||
def m() = f | ||
} | ||
object B { | ||
val f: Int = g() | ||
def g(): Int = f // error | ||
} | ||
object C { | ||
val a: A.type | B.type = if ??? then A else B | ||
def cast[T](a: Any): T = a.asInstanceOf[T] | ||
val c: A.type = cast[A.type](a) // abstraction for c is {A, B} | ||
val d = c.f // treat as c.asInstanceOf[owner of f].f | ||
val e = c.m() // treat as c.asInstanceOf[owner of f].m() | ||
val c2: B.type = cast[B.type](a) | ||
val g = c2.f // no error here | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
class A: | ||
class B(val b: Int) | ||
|
||
object O: | ||
val o: A | Array[Int] = new Array[Int](10) | ||
o match | ||
case a: A => new a.B(10) | ||
case arr: Array[Int] => arr(5) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
class A: | ||
class B(val b: Int) | ||
|
||
object O: | ||
val o: A | (Int => Int) = (x: Int) => x + 1 | ||
o match | ||
case a: A => new a.B(10) | ||
case f: (_ => _) => f.asInstanceOf[Int => Int](5) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
class A: | ||
var x: Int = 10 | ||
|
||
object O: | ||
val o: A | (Int => Int) = (x: Int) => x + 1 | ||
o match | ||
case a: A => a.x = 20 | ||
case f: (_ => _) => f.asInstanceOf[Int => Int](5) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
class A: | ||
var x: Int = 10 | ||
|
||
object O: | ||
val o: A | Array[Int] = new Array[Int](10) | ||
o match | ||
case a: A => a.x = 20 | ||
case arr: Array[Int] => arr(5) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
class A: | ||
var a = 20 | ||
|
||
class B: | ||
var b = 20 | ||
|
||
object O: | ||
val o: A | B = new A | ||
if o.isInstanceOf[A] then | ||
o.asInstanceOf[A].a += 1 | ||
else | ||
o.asInstanceOf[B].b += 1 // o.asInstanceOf[B] is treated as bottom | ||
o match | ||
case o: A => o.a += 1 | ||
case o: B => o.b += 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This case doesn't seem to use the baseclasses. Should all the baseclasses stuff (and the check for empty baseclasses) be pushed down into the second case? Also, perhaps
headOption
is useful on the baseclasses.