Skip to content

Conversation

@Blaisorblade
Copy link
Contributor

In this testcase tp is TermRef(NoPrefix, xs) so widenExpr does nothing, but
widenTermRefExpr is appropriate.

In this testcase `tp` is `TermRef(NoPrefix, xs)` so widenExpr does nothing, but
`widenTermRefExpr` is appropriate.
Since ExprType are unstable, we can keep stable TermRef alone, in case
this provides more info to the pattern matcher/exhaustivity checker.
* generated variable.
*/
private def sanitize(tp: Type): Type = tp.widenExpr match {
private def sanitize(tp: Type): Type = tp.widenIfUnstable match {
Copy link
Contributor

Choose a reason for hiding this comment

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

There is also widenTermRefExpr that seems to work here. Not sure which one is the most appropriate one. I defer to @odersky or @smarter

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah the first commit uses widenTermRefExpr. Maybe @liufengyun can think of examples where widening singletons would make a difference?

Copy link
Member

Choose a reason for hiding this comment

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

What about x match { case _: x.type => ... } ?

Copy link
Contributor

Choose a reason for hiding this comment

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

We concluded with @smarter that there is no need to widen singleton types

* generated variable.
*/
private def sanitize(tp: Type): Type = tp.widenExpr match {
private def sanitize(tp: Type): Type = tp.widenIfUnstable match {
Copy link
Contributor

Choose a reason for hiding this comment

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

We concluded with @smarter that there is no need to widen singleton types

@allanrenucci allanrenucci merged commit 0c21d4c into scala:master Sep 5, 2018
@allanrenucci allanrenucci deleted the fix-4999-pattern-match-byref branch September 5, 2018 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants