-
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
Refactor constant folding of applications #20099
Conversation
Move them in typedApply/typedTypeApply instead of leaving them until adapt. This aligns these folds with folds of uniary operations, which are done already in typedSelect and avoids potentially several calls to ConstFold when arguments are passed to overloaded methods.
5167ce3
to
235c047
Compare
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.
I believe the ConstFold
in TypeAssigner
could now also be removed
scala3/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala
Lines 314 to 316 in 521ce95
def assignType(tree: untpd.TypeApply, fn: Tree, args: List[Tree])(using Context): TypeApply = { | |
def fail = tree.withType(errorType(err.takesNoParamsMsg(fn, "type "), tree.srcPos)) | |
ConstFold(fn.tpe.widen match { |
Otherwise looks good to me !
Not sure about the TypeAssigner one. Directly constructed TypeApply don't go through Applications, so we would stop constant folding them. |
@@ -1189,7 +1190,8 @@ trait Applications extends Compatibility { | |||
case _ => tree.withType(TryDynamicCallType) | |||
} | |||
if (typedFn.tpe eq TryDynamicCallType) tryDynamicTypeApply() | |||
else assignType(cpy.TypeApply(tree)(typedFn, typedArgs), typedFn, typedArgs) | |||
else | |||
ConstFold(assignType(cpy.TypeApply(tree)(typedFn, typedArgs), typedFn, typedArgs)) |
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.
But do we need this one then ? I'm confused about why we need both of them.
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.
Good point. We don't need the one here.
Backports #20099 to the LTS branch. PR submitted by the release tooling. [skip ci]
Move them in typedApply/typedTypeApply instead of leaving them until adapt. This aligns these folds with folds of unary operations, which are done already in typedSelect and avoids potentially several calls to ConstFold when arguments are passed to overloaded methods.