Skip to content

Use == for instance tests against string constants #12799

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

Merged
merged 2 commits into from
Jun 23, 2021

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Jun 12, 2021

We used eq before, as for other objetc singletons. But this means
we are subject to the vagaries of string hashing.

Fixes #12796
Fixes #6996

odersky added 2 commits June 12, 2021 16:05
We used `eq` before, as for other objetc singletons. But this means
we are subject to the vagaries of string hashing.

Fixes scala#12796
@@ -963,8 +963,10 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {

/** `tree.isInstanceOf[tp]`, with special treatment of singleton types */
def isInstance(tp: Type)(using Context): Tree = tp.dealias match {
case ConstantType(c) if c.tag == StringTag =>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we do it for other constants?

Copy link
Contributor Author

@odersky odersky Jun 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only constant types supporting the alternative comparison with eq are classes and strings. Class literals are hash consed, so == and eq are the same. So I believe String is the only type in need of a fix.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To add, I didn't find this issue with integer constants.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Null might also take advantage of this unless some other optimization is already handling it.

@odersky odersky merged commit fb6b453 into scala:master Jun 23, 2021
@odersky odersky deleted the fix-12796 branch June 23, 2021 17:38
@Kordyjan Kordyjan added this to the 3.0.2 milestone Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Union singleton types weirdness with isInstanceOf Inconsistent behavior of String literal type between pattern matching and isInstanceOf
4 participants