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

Make EmptyTuple a case object #14972

Merged

Conversation

nicolasstucki
Copy link
Contributor

No description provided.

@nicolasstucki
Copy link
Contributor Author

Strange, the MiMA passes. I expect it to fail because the case object added new methods on EmptyTuple.

@dwijnand do you know if these definitions are ignored by MiMa for some reason?

@nicolasstucki nicolasstucki added the needs-minor-release This PR cannot be merged until the next minor release label Apr 19, 2022
@dwijnand
Copy link
Member

It looks like we're only checking scala3-library for backwards compatibility. And in backwards compatibility adding methods is only a problem for extendable classes, not for object. So we need to change the setting so both directions are checked:

sbt:scala3> set LocalProject("scala3-library-bootstrapped")/mimaCheckDirection := "both"
[info] Defining scala3-library-bootstrapped / mimaCheckDirection
[info] The new value will be used by scala3-library-bootstrapped / mimaFindBinaryIssues, scala3-library-bootstrapped / mimaReportBinaryIssues
[info] Reapplying settings...
[info] set current project to scala3 (in build file:/d/scala3/)
sbt:scala3> scala3-library-bootstrapped/mimaReportBinaryIssues
[error] scala3-library-bootstrapped: Failed binary compatibility check against org.scala-lang:scala3-library_3:3.1.2! Found 23 potential problems (filtered 11)
[error]  * the type hierarchy of object scala.Tuple#package#EmptyTuple is different in other version. Missing types {scala.deriving.Mirror$Product,scala.deriving.Mirror$Singleton,scala.deriving.Mirror}
[error]    filter with: ProblemFilters.exclude[MissingTypesProblem]("scala.Tuple$package$EmptyTuple$")
...
[error] stack trace is suppressed; run last scala3-library-bootstrapped / mimaReportBinaryIssues for the full output
[error] (scala3-library-bootstrapped / mimaReportBinaryIssues) Failed binary compatibility check against org.scala-lang:scala3-library_3:3.1.2! Found 23 potential problems (filtered 11)
[error] Total time: 0 s, completed 20 Apr 2022, 11:05:16

@nicolasstucki
Copy link
Contributor Author

Thank you @dwijnand.

The check is passing because we updated to 3.2.0-RC1 and the following code chooses BinaryCompatible https://github.com/lampepfl/dotty/blob/main/project/Build.scala#L85-L91. In this case, we only check "backward" and not "both" which is correct behavior.

@nicolasstucki nicolasstucki marked this pull request as ready for review April 22, 2022 08:32
@nicolasstucki nicolasstucki added this to the 3.2.0-RC1 milestone Apr 22, 2022
@nicolasstucki
Copy link
Contributor Author

@sjrd do you see any source or binary compatibility issue with this change? We add some new methods and the possibility of getting a mirror for EmptyTuple.

@nicolasstucki nicolasstucki assigned sjrd and unassigned nicolasstucki May 5, 2022
@nicolasstucki nicolasstucki requested a review from sjrd May 5, 2022 11:11
@sjrd
Copy link
Member

sjrd commented May 5, 2022

It changes the public API, so it's definitely source incompatible (backward and forward).
It's also definitely forward binary incompatible.

It should be backward binary compatible, though.

@nicolasstucki nicolasstucki merged commit f3d72ec into scala:main May 9, 2022
@nicolasstucki nicolasstucki deleted the make-EmptyTuple-a-case-object branch May 9, 2022 13:15
@Kordyjan Kordyjan modified the milestones: 3.2.0-RC1, 3.2.0 Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-minor-release This PR cannot be merged until the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants