-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Respect cacheResult flag in safe initialization cache. #16905
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
Conversation
RE: Signed the Scala CLA. |
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.
Otherwise LGTM.
@@ -893,7 +893,7 @@ object Semantic: | |||
|
|||
case Cold => Cold | |||
|
|||
case ref: Ref => eval(vdef.rhs, ref, enclosingClass) | |||
case ref: Ref => eval(vdef.rhs, ref, enclosingClass, cacheResult = true) |
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.
Please add test cases that motivated the two changes in Semantic.scala
.
@@ -989,7 +989,7 @@ object Semantic: | |||
val errors = Reporter.stopEarly { | |||
val res = { | |||
given Trace = Trace.empty | |||
eval(body, thisV, klass) | |||
eval(body, thisV, klass, cacheResult = true) |
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.
BTW, it seems we miss a test for this one.
The safe initialization checker only needs to cache entries on certain
cachedEval
calls, specified by acacheResult
flag. This PR reduces the size of the safe initialization cache during compilation with-Ysafe-init
by respecting the value of the flag, and only caching when it is true.