-
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
Map class literal constant types #16988
Conversation
97cd4a8
to
7b234f1
Compare
7b234f1
to
92118c4
Compare
case ast.TreeInfo.Impure => "PurityLevel.Impure" | ||
case ast.TreeInfo.PurePath => "PurityLevel.PurePath" | ||
case ast.TreeInfo.IdempotentPath => "PurityLevel.IdempotentPath" | ||
case _ => s"PurityLevel(${x.x})" |
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 was looking at
val countsAsPure =
if dropOp(tree1).symbol.isInlineVal
then isIdempotentExpr(tree1)
else isPureExpr(tree1)
in constToLiteral
and needed to print purity levels.
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.
Could this be achieved by making PurityLevel an enum instead to generate a sensible toString?
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.
Perhaps. But what's a nice way to reimplement min
?
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.
Ah I hadn't considered that, nevermind then.
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.
This could still be implemented as the toString of PurityLevel since it doesn't require a Context.
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.
Sure, it could.
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.
case ast.TreeInfo.Impure => "PurityLevel.Impure" | ||
case ast.TreeInfo.PurePath => "PurityLevel.PurePath" | ||
case ast.TreeInfo.IdempotentPath => "PurityLevel.IdempotentPath" | ||
case _ => s"PurityLevel(${x.x})" |
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.
Could this be achieved by making PurityLevel an enum instead to generate a sensible toString?
Fixes #16954