Skip to content

Document migration steps for changed rules for given prioritisation under 3.6 #20843

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

Closed
WojciechMazur opened this issue Jun 27, 2024 · 8 comments
Assignees
Labels

Comments

@WojciechMazur
Copy link
Contributor

Scala 3.5 introduces gradual changes to rules prioritization #19300 , these now (3.5) warn about selected implicit, and would start yielding errors in the future (3.6).
These changes lead to multiple warnings/errors in real-world projects like lichess-org/lila. We should identify the patterns in which warnings occur and provide a guide/migration steps on how to deal with them. Especially how to deal with changed resolved targets.

@WojciechMazur WojciechMazur added area:documentation stat:needs triage Every issue needs to have an "area" and "itype" label labels Jun 27, 2024
@Gedochao Gedochao added itype:meta Issues about process/similar and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Jun 27, 2024
@soronpo
Copy link
Contributor

soronpo commented Jun 27, 2024

The main issue with this change is that it's a problem that manifests down to the application level. We cannot simply just silence the warnings at the library level. Meaning that any library that experiences this, now needs its own migration guide for its users.

@lenguyenthanh
Copy link

I "successful" migrated lila to 3.5.0-RC3 with -source:3.6-migration: lichess-org/lila#15664. But I'm quite afraid that #19300 will cause a lot of trouble to the community/industry as @soronpo said above.

I believe that there are a lot of projects out there depend on the rule of choosing the most specific implicit (before #19300). Which is kinda intuitive in OOP sense.

I really think that We have to have some migration guide before 3.5 release.

@Gedochao
Copy link
Contributor

cc @odersky

@odersky
Copy link
Contributor

odersky commented Jul 23, 2024

@lenguyenthanh We did tests in the open CB and there was only a tiny minority of projects that broke (something like 5 out of 1500). I was myself very surprised by this and that was the main reason why we decided that we could do the change. After all the change is a huge improvement, without it you cannot really do serious typeclass-based development.

Most of the breakage traced back to Reactive Mongo which is also the root error for Lichess. There were more warnings before but since our latest fixes to avoid spurious warnings it should be much better.

We should do a careful analysis of ReactiveMongo and advise downstream projects of possible breakages. The implicits in Reactive Mongo's BSON handling that cause the switch in priority are unfortunately extremely complex and inscrutable (at least to my eyes).

@Gedochao
Copy link
Contributor

Gedochao commented Aug 5, 2024

@bracevac this would be covered by scala/scala-lang#1675, right?

@lenguyenthanh
Copy link

lenguyenthanh commented Aug 6, 2024

@lenguyenthanh We did tests in the open CB and there was only a tiny minority of projects that broke (something like 5 out of 1500). I was myself very surprised by this and that was the main reason why we decided that we could do the change. After all the change is a huge improvement, without it you cannot really do serious typeclass-based development.

I'm 100% behind this change. It does indeed improve Scala given semantic and I'm very optimistic that this change combining with scala/improvement-proposals#81 It'll easier for new comers understand and use contextual abstraction. Which is one of the power of Scala IMHO.

Most of the breakage traced back to Reactive Mongo which is also the root error for Lichess. There were more warnings before but since our latest fixes to avoid spurious warnings it should be much better.

We should do a careful analysis of ReactiveMongo and advise downstream projects of possible breakages. The implicits in Reactive Mongo's BSON handling that cause the switch in priority are unfortunately extremely complex and inscrutable (at least to my eyes).

My concern is mostly about the migration works for the industry. If the errors is "obvious" like in reactive-mongo, We (the industry) can solve it (maybe somewhat difficult but it's solvable, I solved it in lila by reading the PR and reactive-mongo code). But sometime, We make it compiles, no warning anymore, but the business logic is reversed. And I think this kind of errors is the one We should focus on the migration documentation.

Here are some issues We have had after 3.5.0-RC4 (with given prioritisation change) migrations:

p/s: I agree that the way reactive-mongo and lila using implicit and given are over complicated (I would say "abusing") but I guess that not to abnormal in the industry.

edited to fix typo and add some more information.

@bracevac
Copy link
Contributor

@bracevac this would be covered by scala/scala-lang#1675, right?

Yes. The blog post is live now.

@lenguyenthanh It would be great if you could further minimize your examples and solutions. They’re a bit hard to follow as is, so I’m not sure if there’s something in them that isn’t already covered by our recommendations. If we can boil them down to their essence, we can easily add them to the blog post.

@Gedochao
Copy link
Contributor

scala/scala-lang#1675 has been merged, closing this.
If a follow-up is necessary, feel free to open a new issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants