Skip to content
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

Jump to bottom

Global Initialization checker: Widen values in assignment #21512

Merged
merged 2 commits into from
Sep 3, 2024
Merged

Global Initialization checker: Widen values in assignment #21512

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
533ee8a
Widen values in assignment
liufengyun Aug 29, 2024
76498dc
Address review: Refactor code for clarity
liufengyun Sep 2, 2024
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
45 changes: 26 additions & 19 deletions compiler/src/dotty/tools/dotc/transform/init/Objects.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1226,11 +1226,12 @@ class Objects(using Context @constructorOnly):
extendTrace(id) { evalType(prefix, thisV, klass) }

val value = eval(rhs, thisV, klass)
val widened = widenEscapedValue(value, rhs)

if isLocal then
writeLocal(thisV, lhs.symbol, value)
writeLocal(thisV, lhs.symbol, widened)
else
withTrace(trace2) { assign(receiver, lhs.symbol, value, rhs.tpe) }
withTrace(trace2) { assign(receiver, lhs.symbol, widened, rhs.tpe) }

case closureDef(ddef) =>
Fun(ddef, thisV, klass, summon[Env.Data])
Expand Down Expand Up @@ -1568,6 +1569,28 @@ class Objects(using Context @constructorOnly):
throw new Exception("unexpected type: " + tp + ", Trace:\n" + Trace.show)
}

/** Widen the escaped value (a method argument or rhs of an assignment)
*
* The default widening is 1 for most values, 2 for function values.
* User-specified widening annotations are repected.
*/
def widenEscapedValue(value: Value, expr: Tree): Contextual[Value] =
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest renaming expr to something more specific such as annotatedTree or annotationHost.

expr.tpe.getAnnotation(defn.InitWidenAnnot) match
Copy link
Contributor

Choose a reason for hiding this comment

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

This implements the default as 1 or 2 if there is no annotation, but the default when there is a malformed annotation is always 1.

It would be cleaner to first compute the default widening level (1 or 2), and then do the parsing of the annotation, using that default widening level if annotation parsing fails.

Even cleaner would be a function like:

def parseAnnotation: Option[Int]

that returns None if there is no annotation or there is an annotation that cannot be parsed. Then

parseAnnotation match
  case Some(i) => value.widen(i)
  case None => if ... then value.widen(2) else value.widen(1)

case Some(annot) =>
annot.argument(0).get match
case arg @ Literal(c: Constants.Constant) =>
val height = c.intValue
if height < 0 then
report.warning("The argument should be positive", arg)
value.widen(1)
else
value.widen(c.intValue)
case arg =>
report.warning("The argument should be a constant integer value", arg)
value.widen(1)
case _ =>
if value.isInstanceOf[Fun] then value.widen(2) else value.widen(1)

/** Evaluate arguments of methods and constructors */
def evalArgs(args: List[Arg], thisV: ThisValue, klass: ClassSymbol): Contextual[List[ArgInfo]] =
val argInfos = new mutable.ArrayBuffer[ArgInfo]
Expand All @@ -1578,23 +1601,7 @@ class Objects(using Context @constructorOnly):
else
eval(arg.tree, thisV, klass)

val widened =
arg.tree.tpe.getAnnotation(defn.InitWidenAnnot) match
case Some(annot) =>
annot.argument(0).get match
case arg @ Literal(c: Constants.Constant) =>
val height = c.intValue
if height < 0 then
report.warning("The argument should be positive", arg)
res.widen(1)
else
res.widen(c.intValue)
case arg =>
report.warning("The argument should be a constant integer value", arg)
res.widen(1)
case _ =>
if res.isInstanceOf[Fun] then res.widen(2) else res.widen(1)

val widened = widenEscapedValue(res, arg.tree)
argInfos += ArgInfo(widened, trace.add(arg.tree), arg.tree)
}
argInfos.toList
Expand Down