-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix: #23261 Distinguish 0.0 and -0.0 in ConstantType match types #23265
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?
Conversation
@@ -401,7 +401,13 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling | |||
compareErasedValueType | |||
case ConstantType(v2) => | |||
tp1 match { | |||
case ConstantType(v1) => v1.value == v2.value && recur(v1.tpe, v2.tpe) |
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.
MEMO
I tried to modify the Constants.scala below to change the processing of the ==
method, but it didn't work.
override def equals(other: Any): Boolean = other match { |
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.
The line should use equals, which is already correct, see equalHashValue comment.
case ConstantType(v1) => v1.value.equals(v2.value) && recur(v1.tpe, v2.tpe)
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.
sorry, second look says
v1 == v2 && recur(v1.tpe, v2.tpe)
Do not unwrap Constant to underlying value, use equals on Constant.
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.
The internal tag
in Constant is already checked for equality. Then recur(v1.tpe, v2.tpe)
is not needed.
However, what about ClazzTag
where Constant
wraps a Type
?
In general, the equals
check is wrong. But if the underlying type is always some normalized class type, and it's comparing invariant Class[C]
, then the existing v1.value == v2.value
is correct (and v1 == v2
is also correct).
tests/neg/i23261.scala
Outdated
@@ -0,0 +1,7 @@ | |||
@main def main(): Unit = | |||
summon[0.0 =:= -0.0] |
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.
summon[0.0 =:= -0.0] // error
to annotate an expected error on that line
- Ensure match types distinguish between 0.0 and -0.0 in pattern matching. - This enables accurate reduction in match types involving Double constants. Close scala#23261
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 suspect the call to recur
is extraneous and could be deleted, but it must also be correct to leave it as it is. (It is no less correct than the existing code.)
@@ -401,7 +401,13 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling | |||
compareErasedValueType | |||
case ConstantType(v2) => | |||
tp1 match { | |||
case ConstantType(v1) => v1.value == v2.value && recur(v1.tpe, v2.tpe) |
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.
The internal tag
in Constant is already checked for equality. Then recur(v1.tpe, v2.tpe)
is not needed.
However, what about ClazzTag
where Constant
wraps a Type
?
In general, the equals
check is wrong. But if the underlying type is always some normalized class type, and it's comparing invariant Class[C]
, then the existing v1.value == v2.value
is correct (and v1 == v2
is also correct).
Fixes #23261