Skip to content

Fix remaining initialization warnings in bootstrapping #15682

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.

Sign up for GitHub

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 12 commits into from
Jul 17, 2022
Merged

Fix remaining initialization warnings in bootstrapping #15682

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
c427046
Add test
liufengyun Jul 14, 2022
d7754d9
Refine error message
liufengyun Jul 14, 2022
3d28731
Use hot instead of fully initialized
liufengyun Jul 14, 2022
b6af7ee
Update check files
liufengyun Jul 14, 2022
80d9975
Fix crash in member selection from type cast
liufengyun Jul 14, 2022
920d4a9
Code refactoring
liufengyun Jul 14, 2022
41a4f71
Faster promotion
liufengyun Jul 14, 2022
dd1e724
Reclassify test
liufengyun Jul 14, 2022
b7dba0d
Rename variable
liufengyun Jul 14, 2022
4a4affd
Add assertions to tryPromote
liufengyun Jul 15, 2022
0ae7346
Fix typo in doc
liufengyun Jul 15, 2022
df875ee
Improve error messages
liufengyun Jul 15, 2022
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
90 changes: 46 additions & 44 deletions compiler/src/dotty/tools/dotc/transform/init/Errors.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,55 +18,57 @@ object Errors:

def pos(using Context): SourcePosition = trace.last.sourcePos

def stacktrace(preamble: String = " Calling trace:\n")(using Context): String = buildStacktrace(trace, preamble)

def issue(using Context): Unit =
report.warning(show, this.pos)
end Error

def buildStacktrace(trace: Seq[Tree], preamble: String)(using Context): String = if trace.isEmpty then "" else preamble + {
var lastLineNum = -1
var lines: mutable.ArrayBuffer[String] = new mutable.ArrayBuffer
trace.foreach { tree =>
val pos = tree.sourcePos
val prefix = "-> "
val line =
if pos.source.exists then
val loc = "[ " + pos.source.file.name + ":" + (pos.line + 1) + " ]"
val code = SyntaxHighlighting.highlight(pos.lineContent.trim.nn)
i"$code\t$loc"
else
tree.show
val positionMarkerLine =
if pos.exists && pos.source.exists then
positionMarker(pos)
else ""

def stacktrace(preamble: String = " Calling trace:\n")(using Context): String = if trace.isEmpty then "" else preamble + {
var lastLineNum = -1
var lines: mutable.ArrayBuffer[String] = new mutable.ArrayBuffer
trace.foreach { tree =>
val pos = tree.sourcePos
val prefix = "-> "
val line =
if pos.source.exists then
val loc = "[ " + pos.source.file.name + ":" + (pos.line + 1) + " ]"
val code = SyntaxHighlighting.highlight(pos.lineContent.trim.nn)
i"$code\t$loc"
else
tree.show
val positionMarkerLine =
if pos.exists && pos.source.exists then
positionMarker(pos)
else ""

// always use the more precise trace location
if lastLineNum == pos.line then
lines.dropRightInPlace(1)

lines += (prefix + line + "\n" + positionMarkerLine)

lastLineNum = pos.line
}
val sb = new StringBuilder
for line <- lines do sb.append(line)
sb.toString
// always use the more precise trace location
if lastLineNum == pos.line then
lines.dropRightInPlace(1)

lines += (prefix + line + "\n" + positionMarkerLine)

lastLineNum = pos.line
}
val sb = new StringBuilder
for line <- lines do sb.append(line)
sb.toString
}

/** Used to underline source positions in the stack trace
* pos.source must exist
*/
private def positionMarker(pos: SourcePosition): String =
val trimmed = pos.lineContent.takeWhile(c => c.isWhitespace).length
val padding = pos.startColumnPadding.substring(trimmed).nn + " "
val carets =
if (pos.startLine == pos.endLine)
"^" * math.max(1, pos.endColumn - pos.startColumn)
else "^"
/** Used to underline source positions in the stack trace
* pos.source must exist
*/
private def positionMarker(pos: SourcePosition): String =
val trimmed = pos.lineContent.takeWhile(c => c.isWhitespace).length
val padding = pos.startColumnPadding.substring(trimmed).nn + " "
val carets =
if (pos.startLine == pos.endLine)
"^" * math.max(1, pos.endColumn - pos.startColumn)
else "^"

s"$padding$carets\n"
s"$padding$carets\n"

override def toString() = this.getClass.getName.nn
end Error
override def toString() = this.getClass.getName.nn

/** Access non-initialized field */
case class AccessNonInit(field: Symbol, trace: Seq[Tree]) extends Error:
Expand Down Expand Up @@ -97,7 +99,7 @@ object Errors:
case class UnsafePromotion(msg: String, trace: Seq[Tree], error: Error) extends Error:
def show(using Context): String =
msg + stacktrace() + "\n" +
"Promoting the value to fully initialized failed due to the following problem:\n" +
"Promoting the value to hot failed due to the following problem:\n" +
error.show

/** Unsafe leaking a non-hot value as constructor arguments
Expand Down Expand Up @@ -129,5 +131,5 @@ object Errors:
acc + text2
}
val verb = if multiple then " are " else " is "
val adjective = "not fully initialized."
val adjective = "not hot."
subject + verb + adjective
107 changes: 70 additions & 37 deletions compiler/src/dotty/tools/dotc/transform/init/Semantic.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,9 @@ object Semantic:
def add(node: Tree): Trace = trace :+ node
def toVector: Vector[Tree] = trace

def show(using trace: Trace, ctx: Context): String = buildStacktrace(trace, "\n")

def position(using trace: Trace): Tree = trace.last
type Trace = Trace.Trace

import Trace.*
Expand Down Expand Up @@ -652,7 +655,7 @@ object Semantic:
value.promote(msg)
value

def select(field: Symbol, needResolve: Boolean = true): Contextual[Value] = log("select " + field.show + ", this = " + value, printer, (_: Value).show) {
def select(field: Symbol, receiver: Type, needResolve: Boolean = true): Contextual[Value] = log("select " + field.show + ", this = " + value, printer, (_: Value).show) {
if promoted.isCurrentObjectPromoted then Hot
else value match {
case Hot =>
Expand All @@ -668,7 +671,7 @@ object Semantic:
if target.is(Flags.Lazy) then
val rhs = target.defTree.asInstanceOf[ValDef].rhs
eval(rhs, ref, target.owner.asClass, cacheResult = true)
else
else if target.exists then
val obj = ref.objekt
if obj.hasField(target) then
obj.field(target)
Expand All @@ -691,13 +694,21 @@ object Semantic:
val error = AccessNonInit(target, trace.toVector)
reporter.report(error)
Hot
else
if ref.klass.isSubClass(receiver.widenSingleton.classSymbol) then
report.error("[Internal error] Unexpected resolution failure: ref.klass = " + ref.klass.show + ", field = " + field.show + Trace.show, Trace.position)
Hot
else
// This is possible due to incorrect type cast.
// See tests/init/pos/Type.scala
Hot

case fun: Fun =>
report.error("[Internal error] unexpected tree in selecting a function, fun = " + fun.expr.show, fun.expr)
report.error("[Internal error] unexpected tree in selecting a function, fun = " + fun.expr.show + Trace.show, fun.expr)
Hot

case RefSet(refs) =>
refs.map(_.select(field)).join
refs.map(_.select(field, receiver)).join
}
}

Expand Down Expand Up @@ -809,13 +820,21 @@ object Semantic:
val error = CallUnknown(target, trace.toVector)
reporter.report(error)
Hot
else
else if target.exists then
// method call resolves to a field
val obj = ref.objekt
if obj.hasField(target) then
obj.field(target)
else
value.select(target, needResolve = false)
value.select(target, receiver, needResolve = false)
else
if ref.klass.isSubClass(receiver.widenSingleton.classSymbol) then
report.error("Unexpected resolution failure: ref.klass = " + ref.klass.show + ", meth = " + meth.show + Trace.show, Trace.position)
Hot
else
// This is possible due to incorrect type cast.
// See tests/init/pos/Type.scala
Hot

case Fun(body, thisV, klass) =>
// meth == NoSymbol for poly functions
Expand All @@ -840,7 +859,7 @@ object Semantic:

value match {
case Hot | Cold | _: RefSet | _: Fun =>
report.error("unexpected constructor call, meth = " + ctor + ", value = " + value, trace.toVector.last)
report.error("[Internal error] unexpected constructor call, meth = " + ctor + ", value = " + value + Trace.show, Trace.position)
Hot

case ref: Warm if ref.isPopulatingParams =>
Expand Down Expand Up @@ -947,7 +966,7 @@ object Semantic:
warm

case Fun(body, thisV, klass) =>
report.error("[Internal error] unexpected tree in instantiating a function, fun = " + body.show, trace.toVector.last)
report.error("[Internal error] unexpected tree in instantiating a function, fun = " + body.show + Trace.show, Trace.position)
Hot

case RefSet(refs) =>
Expand All @@ -967,7 +986,7 @@ object Semantic:
case Hot => Hot
case ref: Ref => ref.objekt.field(sym)
case _ =>
report.error("[Internal error] unexpected this value accessing local variable, sym = " + sym.show + ", thisValue = " + thisValue2.show, trace.toVector.last)
report.error("[Internal error] unexpected this value accessing local variable, sym = " + sym.show + ", thisValue = " + thisValue2.show + Trace.show, Trace.position)
Hot
else if sym.is(Flags.Param) then
Hot
Expand All @@ -985,7 +1004,7 @@ object Semantic:
case ref: Ref => eval(vdef.rhs, ref, enclosingClass)

case _ =>
report.error("[Internal error] unexpected this value when accessing local variable, sym = " + sym.show + ", thisValue = " + thisValue2.show, trace.toVector.last)
report.error("[Internal error] unexpected this value when accessing local variable, sym = " + sym.show + ", thisValue = " + thisValue2.show + Trace.show, Trace.position)
Hot
end match

Expand Down Expand Up @@ -1081,7 +1100,7 @@ object Semantic:
eval(body, thisV, klass)
}
given Trace = Trace.empty.add(body)
res.promote("The function return value is not fully initialized. Found = " + res.show + ". ")
res.promote("The function return value is not hot. Found = " + res.show + ". ")
}
if errors.nonEmpty then
reporter.report(UnsafePromotion(msg, trace.toVector, errors.head))
Expand Down Expand Up @@ -1119,20 +1138,34 @@ object Semantic:

val errors = Reporter.stopEarly {
for klass <- warm.klass.baseClasses if klass.hasSource do
for member <- klass.info.decls do
if !member.isType && !member.isConstructor && member.hasSource && !member.is(Flags.Deferred) then
if member.is(Flags.Method, butNot = Flags.Accessor) then
withTrace(Trace.empty) {
val args = member.info.paramInfoss.flatten.map(_ => ArgInfo(Hot, Trace.empty))
val res = warm.call(member, args, receiver = NoType, superType = NoType)
res.promote("Cannot prove that the return value of " + member.show + " is fully initialized. Found = " + res.show + ". ")
}
else
withTrace(Trace.empty) {
val res = warm.select(member)
res.promote("Cannot prove that the field " + member.show + " is fully initialized. Found = " + res.show + ". ")
}
end for
val obj = warm.objekt
val outer = obj.outer(klass)
val ctor = klass.primaryConstructor
val isHotSegment = outer.isHot && {
val ctorDef = ctor.defTree.asInstanceOf[DefDef]
val params = ctorDef.termParamss.flatten.map(_.symbol)
// We have cached all parameters on the object
params.forall(param => obj.field(param).isHot)
}

// If the outer and parameters of a class are all hot, then accessing fields and methods of the current
// segment of the object should be OK. They may only create problems via virtual method calls on `this`, but
// those methods are checked as part of the check for the class where they are defined.
if !isHotSegment then
for member <- klass.info.decls do
if !member.isType && !member.isConstructor && member.hasSource && !member.is(Flags.Deferred) then
if member.is(Flags.Method, butNot = Flags.Accessor) then
withTrace(Trace.empty) {
val args = member.info.paramInfoss.flatten.map(_ => ArgInfo(Hot, Trace.empty))
val res = warm.call(member, args, receiver = warm.klass.typeRef, superType = NoType)
res.promote("Cannot prove that the return value of " + member.show + " is hot. Found = " + res.show + ". ")
}
else
withTrace(Trace.empty) {
val res = warm.select(member, receiver = warm.klass.typeRef)
res.promote("Cannot prove that the field " + member.show + " is hot. Found = " + res.show + ". ")
}
end for
end for
}

Expand Down Expand Up @@ -1231,7 +1264,7 @@ object Semantic:
/** Utility definition used for better error-reporting of argument errors */
case class ArgInfo(value: Value, trace: Trace):
def promote: Contextual[Unit] = withTrace(trace) {
value.promote("Cannot prove the argument is fully initialized. Only fully initialized values are safe to leak.\nFound = " + value.show + ". ")
value.promote("Cannot prove the method argument is hot. Only hot values are safe to leak.\nFound = " + value.show + ". ")
}

/** Evaluate an expression with the given value for `this` in a given class `klass`
Expand Down Expand Up @@ -1344,7 +1377,7 @@ object Semantic:
resolveThis(target, qual, current.asClass)
}
case _ =>
withTrace(trace2) { qual.select(expr.symbol) }
withTrace(trace2) { qual.select(expr.symbol, receiver = qualifier.tpe) }

case _: This =>
cases(expr.tpe, thisV, klass)
Expand All @@ -1368,12 +1401,12 @@ object Semantic:
eval(qual, thisV, klass)
val res = eval(rhs, thisV, klass)
extendTrace(expr) {
res.ensureHot("The RHS of reassignment must be fully initialized. Found = " + res.show + ". ")
res.ensureHot("The RHS of reassignment must be hot. Found = " + res.show + ". ")
}
case id: Ident =>
val res = eval(rhs, thisV, klass)
extendTrace(expr) {
res.ensureHot("The RHS of reassignment must be fully initialized. Found = " + res.show + ". ")
res.ensureHot("The RHS of reassignment must be hot. Found = " + res.show + ". ")
}

case closureDef(ddef) =>
Expand All @@ -1396,14 +1429,14 @@ object Semantic:
case Match(selector, cases) =>
val res = eval(selector, thisV, klass)
extendTrace(selector) {
res.ensureHot("The value to be matched needs to be fully initialized. Found = " + res.show + ". ")
res.ensureHot("The value to be matched needs to be hot. Found = " + res.show + ". ")
}
eval(cases.map(_.body), thisV, klass).join

case Return(expr, from) =>
val res = eval(expr, thisV, klass)
extendTrace(expr) {
res.ensureHot("return expression must be fully initialized. Found = " + res.show + ". ")
res.ensureHot("return expression must be hot. Found = " + res.show + ". ")
}

case WhileDo(cond, body) =>
Expand Down Expand Up @@ -1453,7 +1486,7 @@ object Semantic:
Hot

case _ =>
report.error("[Internal error] unexpected tree", expr)
report.error("[Internal error] unexpected tree" + Trace.show, expr)
Hot

/** Handle semantics of leaf nodes */
Expand All @@ -1466,7 +1499,7 @@ object Semantic:
thisV.accessLocal(tmref, klass)

case tmref: TermRef =>
cases(tmref.prefix, thisV, klass).select(tmref.symbol)
cases(tmref.prefix, thisV, klass).select(tmref.symbol, receiver = tmref.prefix)

case tp @ ThisType(tref) =>
val cls = tref.classSymbol.asClass
Expand All @@ -1482,7 +1515,7 @@ object Semantic:
Hot

case _ =>
report.error("[Internal error] unexpected type " + tp, trace.toVector.last)
report.error("[Internal error] unexpected type " + tp + Trace.show, Trace.position)
Hot
}

Expand All @@ -1497,15 +1530,15 @@ object Semantic:
val obj = ref.objekt
val outerCls = klass.owner.lexicallyEnclosingClass.asClass
if !obj.hasOuter(klass) then
val error = PromoteError("[Internal error] outer not yet initialized, target = " + target + ", klass = " + klass + ", object = " + obj, trace.toVector)
report.error(error.show, trace.toVector.last)
val error = "[Internal error] outer not yet initialized, target = " + target + ", klass = " + klass + ", object = " + obj + Trace.show
report.error(error, Trace.position)
Hot
else
resolveThis(target, obj.outer(klass), outerCls)
case RefSet(refs) =>
refs.map(ref => resolveThis(target, ref, klass)).join
case fun: Fun =>
report.error("[Internal error] unexpected thisV = " + thisV + ", target = " + target.show + ", klass = " + klass.show, trace.toVector.last)
report.error("[Internal error] unexpected thisV = " + thisV + ", target = " + target.show + ", klass = " + klass.show + Trace.show, Trace.position)
Cold
case Cold => Cold

Expand Down
Loading