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

Fix casting of generic tuple selection #14590

Merged
merged 6 commits into from
Mar 15, 2022

Conversation

dwijnand
Copy link
Member

@dwijnand dwijnand commented Mar 1, 2022

Obviously, in hindsight, we can't use the types of the unapply arguments
as the type for selecting the components of a (generic) tuple...

Fixes #14587

[test_non_bootstrapped]

@dwijnand dwijnand marked this pull request as draft March 1, 2022 11:50
@dwijnand dwijnand assigned dwijnand and unassigned nicolasstucki Mar 1, 2022
@michelou
Copy link
Contributor

michelou commented Mar 1, 2022

@dwijnand 👍

@oscar-broman
Copy link

Perhaps not directly related to this, but the type inferred for bar below is Some[(Some[Int], String) | (None.type, String)]. Should it not be Some[(Option[Int], String)]?

  def foo: String
  val bar = {
    if (foo.isEmpty) Some((Some(1), ""))
    else Some((None, ""))
  }

@dwijnand
Copy link
Member Author

dwijnand commented Mar 1, 2022

Perhaps not directly related to this, but the type inferred for bar below is Some[(Some[Int], String) | (None.type, String)]. Should it not be Some[(Option[Int], String)]?

The former reduces to the latter but that only happens when and if it needs to. What difference does it make to you? (Which sounds miles more aggressive than I mean it... 😅 )

@oscar-broman
Copy link

Nothing at all, only thing I can think of is some obscure cases where it might make things harder for type class derivation.

I just noticed it when trying this out in Scala 2 and just wanted to be sure nothing was overlooked.

@dwijnand dwijnand force-pushed the patmat-fix-generic-tuple-casting branch 2 times, most recently from d684972 to 2f19e30 Compare March 2, 2022 08:45
@dwijnand
Copy link
Member Author

dwijnand commented Mar 7, 2022

[error] ## Exception when compiling 56 sources to /__w/dotty/dotty/community-build/community-projects/scodec/unitTests/target/scala-3.1.3-RC1-bin-SNAPSHOT/test-classes
[error] java.lang.AssertionError: assertion failed: Failed to find type at index 1 in (Option[Int], Option[Int], Option[Int])
[error] scala.Predef$Ensuring$.ensuring$extension(Predef.scala:279)
[error] dotty.tools.dotc.transform.PatternMatcher$Translator.tupleApp$1(PatternMatcher.scala:335)

Obviously, in hindsight, we can't use the types of the unapply arguments
as the type for selecting the components of a (generic) tuple...
@dwijnand dwijnand force-pushed the patmat-fix-generic-tuple-casting branch from 7813fe9 to 83e0288 Compare March 8, 2022 16:41
@dwijnand dwijnand assigned nicolasstucki and unassigned dwijnand Mar 9, 2022
@dwijnand dwijnand marked this pull request as ready for review March 9, 2022 08:41
Copy link
Contributor

@nicolasstucki nicolasstucki left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM

@dwijnand dwijnand enabled auto-merge March 14, 2022 15:53
@dwijnand dwijnand disabled auto-merge March 14, 2022 20:44
@dwijnand dwijnand force-pushed the patmat-fix-generic-tuple-casting branch from 094c963 to 08889fe Compare March 15, 2022 08:56
@dwijnand dwijnand enabled auto-merge March 15, 2022 08:56
@nicolasstucki
Copy link
Contributor

Doc tests failed due to a time out. I restarted the tests.

@dwijnand dwijnand merged commit 9cbee41 into scala:main Mar 15, 2022
@dwijnand dwijnand deleted the patmat-fix-generic-tuple-casting branch March 15, 2022 10:37
@Kordyjan Kordyjan added this to the 3.1.3 milestone Aug 1, 2023
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.

ClassCastException from simple use of pattern matching
6 participants