-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix errors in the global initialization checker when compiling bootstrapped dotty #23429
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
base: main
Are you sure you want to change the base?
Fix errors in the global initialization checker when compiling bootstrapped dotty #23429
Conversation
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.
LGTM
@@ -151,12 +151,12 @@ class Objects(using Context @constructorOnly): | |||
def hasVar(sym: Symbol)(using Heap.MutableData): Boolean = Heap.containsVal(this, sym) | |||
|
|||
def initVal(field: Symbol, value: Value)(using Context, Heap.MutableData) = log("Initialize " + field.show + " = " + value + " for " + this, printer) { | |||
assert(!field.is(Flags.Mutable), "Field is mutable: " + field.show) | |||
assert(field.is(Flags.Param) || !field.is(Flags.Mutable), "Field is mutable: " + field.show) | |||
Heap.writeJoinVal(this, field, value) | |||
} |
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.
It is not obvious why Flags.Param
is an exception. A comment will be helpful.
targetType.asInstanceOf[ExprType].resType | ||
else | ||
targetType.asInstanceOf[MethodType].resType | ||
val returnType = targetType.finalResultType |
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.
Nice simplification 👍
@@ -1326,7 +1318,8 @@ class Objects(using Context @constructorOnly): | |||
report.warning("[Internal error] top-level class should have `Package` as outer, class = " + klass.show + ", outer = " + outer.show + ", " + Trace.show, Trace.position) | |||
(Bottom, Env.NoEnv) | |||
else | |||
val outerCls = klass.owner.enclosingClass.asClass | |||
// enclosingClass is specially handled for java static terms, so use `lexicallyEnclosingClass` here | |||
val outerCls = klass.owner.lexicallyEnclosingClass.asClass |
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.
Nice catch and comment 👍
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.
LGTM also except for a comment about why Flags.Param
is an exception.
With this fix, the global initialization checker can be run when compiling bootstrapped dotty and reported warnings in the dotty source code
[test_scala2_library_tasty]