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

Irrefutable for generators don't require withFilter under -source:future #15593

Merged
merged 1 commit into from
Jul 6, 2022

Conversation

griggt
Copy link
Contributor

@griggt griggt commented Jul 5, 2022

Fixes #15579

@griggt griggt added the backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it. label Jul 5, 2022
@griggt griggt requested a review from bishabosha July 5, 2022 19:57
@griggt griggt added this to the 3.2.0 backports milestone Jul 5, 2022
Copy link
Member

@bishabosha bishabosha left a comment

Choose a reason for hiding this comment

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

Edit: ignore

@bishabosha bishabosha dismissed their stale review July 6, 2022 08:48

misunderstanding

Copy link
Member

@bishabosha bishabosha left a comment

Choose a reason for hiding this comment

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

Approving, then the future behaviour should become the standard in 3.3.

I was a bit confused originally due to the warning messages for refutable patterns, for example, in 3.2.0-RC1 (and source 3.2) the warning for a refutable pattern suggests that withFilter is going to be added only with case, which implies that for a irrefutable pattern there would be no withFilter:

scala> for (x, y) <- List(('a', 23): Tuple) yield (y, x)
1 warning found
-- Warning: --------------------------------------------------------------------
1 |for (x, y) <- List(('a', 23): Tuple) yield (y, x)
  |    ^^^^^^
  |pattern's type (Any, Any) is more specialized than the right hand side expression's type Tuple
  |
  |If the narrowing is intentional, this can be communicated by adding the `case` keyword before the full pattern,
  |which will result in a filtering for expression (using `withFilter`).
val res0: List[(Any, Any)] = List((23,a))

perhaps the warning message should be slightly different for 3.2 to make it clear it is always filtering.

@bishabosha bishabosha merged commit ddabbe6 into scala:main Jul 6, 2022
@bishabosha bishabosha mentioned this pull request Jul 6, 2022
11 tasks
@Kordyjan Kordyjan added backport:accepted This PR needs to be backported, once it's been backported replace this tag by "backport:done" and removed backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it. labels Jul 6, 2022
@griggt griggt deleted the fix-15579 branch July 6, 2022 15:20
bishabosha added a commit that referenced this pull request Jul 6, 2022
@griggt griggt added backport:done This PR was successfully backported. and removed backport:accepted This PR needs to be backported, once it's been backported replace this tag by "backport:done" labels Jul 7, 2022
@Andrapyre
Copy link

@griggt and @bishabosha , will this become the standard in 3.3? Currently on 3.3.0-RC2, tuple decomposition is still failing when using cats effect.

So the below would fail with a withFilter error:

val someIOOp: IO[(Int, String)] = ???

for {
   (num, str) <- someIOOp
} yield ()

@bishabosha
Copy link
Member

Not currently, the PR for that was stuck in porting the community build. Is there an issue for this tuple decomposition bug?

@Andrapyre
Copy link

@bishabosha , see here

@Kordyjan Kordyjan modified the milestones: 3.2.0 backports, 3.2.1 Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:done This PR was successfully backported.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

withFilter required for irrefutable for comprehension pattern
4 participants