-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Conversation
|
||
def main(args: Array[String]): Unit = { | ||
isAType("a") | ||
println(new String("a").isInstanceOf["a"]) |
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 argue in #6996 that this should return true.
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.
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"`; |
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 don't understand why we need to handle case _: "a"
specially but not case x: "a"
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.
Oh, I guess because the latter is desugared to case x @ (_: "a")
? This is all pretty subtle :/
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.
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. |
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.
@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.
@smarter @AleksanderBG Can we get a resolution on this so that we can close or merge for the release? |
I made the resolution myself. LGTM! |
This will need more work, see my comment in #6996 (comment). |
Fixes #6996.
We accidentally constant-folded wildcards in typecase pattern which tested for singleton types, which made their behaviour inconsistent with normal type checks.