Skip to content

Make singleton typecase patterns and type checks consistent #7027

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 3 commits into from
Dec 12, 2019

Conversation

abgruszecki
Copy link
Contributor

Fixes #6996.

We accidentally constant-folded wildcards in typecase pattern which tested for singleton types, which made their behaviour inconsistent with normal type checks.


def main(args: Array[String]): Unit = {
isAType("a")
println(new String("a").isInstanceOf["a"])
Copy link
Member

Choose a reason for hiding this comment

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

I argue in #6996 that this should return true.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Regardless of the result of that discussion, I think we can merge this PR. It's unlikely that we want FirstTransform to accidentally mangle some patterns.

toTypeTree(tree)
} else if (tree.name != nme.WILDCARD) {
// We want to constant-fold _some_ idents here - for instance, @switch needs to see literals in patterns.
// However, constant-foldable wildcards can occur in patterns, for instance as `case _: "a"`;
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand why we need to handle case _: "a" specially but not case x: "a"

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I guess because the latter is desugared to case x @ (_: "a") ? This is all pretty subtle :/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Argh, I see where the comment is hilariously unclear. Give me a second.

// AFAIK, constant-foldable wildcard idents can only occur in patterns, for instance as `case _: "a"`.
// Constant-folding that would result in `case "a": "a"`, which changes the meaning of the pattern.
// Note that we _do_ want to constant-fold idents in patterns that _aren't_ wildcards -
// for example, @switch annotation needs to see inlined literals and not indirect references.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@smarter I think this comment is at the precise level of detail that we want. I don't think going into the subtle differences of Bind/Ident nodes in patterns makes sense - if we did that consequently in the whole compiler, then more than half of the documentation would be permanently outdated.

@odersky
Copy link
Contributor

odersky commented Dec 12, 2019

@smarter @AleksanderBG Can we get a resolution on this so that we can close or merge for the release?

@odersky
Copy link
Contributor

odersky commented Dec 12, 2019

I made the resolution myself. LGTM!

@odersky odersky merged commit 6261ed2 into scala:master Dec 12, 2019
@odersky odersky deleted the i6996 branch December 12, 2019 17:04
@smarter
Copy link
Member

smarter commented Dec 12, 2019

This will need more work, see my comment in #6996 (comment).

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.

Inconsistent behavior of String literal type between pattern matching and isInstanceOf
3 participants