Skip to content
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

Exhaustivity: don't widen & then warn on singleton types #9209

Merged
merged 1 commit into from Sep 24, 2020

Conversation

dwijnand
Copy link
Member

@scala-jenkins scala-jenkins added this to the 2.13.4 milestone Sep 16, 2020
@dwijnand
Copy link
Member Author

dwijnand commented Sep 16, 2020

An ambitious attempt at "just" shuffling some widen's around... but it didn't seem to produce unwanted effects after a quick partest --update-check --grep patmat run. So let's see what the whole suite (+ bootstrapped) says...

@dwijnand dwijnand force-pushed the exhaust-literal-type-false-positive branch from ac63c93 to 13e3b96 Compare September 17, 2020 09:16
@dwijnand dwijnand marked this pull request as ready for review September 17, 2020 16:36
Copy link
Member

@lrytz lrytz left a comment

Choose a reason for hiding this comment

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

Not so easy for me to really review as I barely know anything about the patamat implementation. Generally it looks fine, the check file updates look OK. The setInfo down in emitSwitch is a little suspicious.

@dwijnand dwijnand force-pushed the exhaust-literal-type-false-positive branch 2 times, most recently from 1c93b0e to 2fe421a Compare September 22, 2020 18:17
Copy link
Member

@lrytz lrytz left a comment

Choose a reason for hiding this comment

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

Very nice!

val Match(Typed(selTree: Ident, _), cases) = sw
val sel = selTree.symbol
val Match(Typed(selTree, _), cases) = sw
val selArg = selTree match {
Copy link
Member

@lrytz lrytz Sep 23, 2020

Choose a reason for hiding this comment

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

I'm not sure if this should be a def to avoid structural sharing in the AST. Since trees have mutable fields (.tpe, .pos) this might be a problem - I'm really not sure if it matters in practice. @retronym what's your intuition?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh! Wow, yeah, good catch, thanks. Let's do that.

@dwijnand dwijnand force-pushed the exhaust-literal-type-false-positive branch from 2fe421a to e2a36e7 Compare September 23, 2020 15:03
@dwijnand dwijnand force-pushed the exhaust-literal-type-false-positive branch from e2a36e7 to 1becce1 Compare September 24, 2020 07:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants