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 alleycats Set Functor ambiguous implicits #4678

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

LaurenceWarne
Copy link

Hi, on 2.12.0 I find that I get the error:

ambiguous implicit values:
  both value alleyCatsStdSetMonad in trait SetInstances of type cats.Monad[Set] with cats.Alternative[Set]
  and value alleyCatsSetTraverse in trait SetInstances of type cats.Traverse[Set]
  match expected type cats.Functor[Set] (lsp)

With for example the setup:

import alleycats.std.set._

object test {
  implicitly[cats.Functor[Set]]
}

To try and fix I've used the approach here: https://typelevel.org/cats/guidelines.html#implicit-instance-priority, which fixes the original problem though introduces binary compatibility issues which I don't know how to fix!:

[error]  * abstract synthetic method alleycats$std$SetInstances0$_setter_$alleyCatsStdSetMonad_=(cats.Monad)Unit in interface alleycats.std.SetInstances0 is inherited by class SetInstances in current version.
[error]    filter with: ProblemFilters.exclude[InheritedNewAbstractMethodProblem]("alleycats.std.SetInstances.alleycats$std$SetInstances0$_setter_$alleyCatsStdSetMonad_=")
[error]  * abstract synthetic method alleycats$std$SetInstances1$_setter_$alleyCatsSetTraverse_=(cats.Traverse)Unit in interface alleycats.std.SetInstances1 is inherited by class SetInstances in current version.
[error]    filter with: ProblemFilters.exclude[InheritedNewAbstractMethodProblem]("alleycats.std.SetInstances.alleycats$std$SetInstances1$_setter_$alleyCatsSetTraverse_=")

Suggestions very welcome! Thanks

@satorg
Copy link
Contributor

satorg commented Nov 24, 2024

@LaurenceWarne , thank you for your help!

Regarding the original issue:

ambiguous implicit values:
both value alleyCatsStdSetMonad in trait SetInstances of type cats.Monad[Set] with cats.Alternative[Set]
and value alleyCatsSetTraverse in trait SetInstances of type cats.Traverse[Set]
match expected type cats.Functor[Set] (lsp)

In theory, all these instances (i.e., Traverse, Monad, Alternative, etc) can be (and usually are) gathered in a single implementation class and therefore exposed by a single implicit val. For instance, see

implicit val catsStdInstancesForList
: Traverse[List] with Alternative[List] with Monad[List] with CoflatMap[List] with Align[List] =
new Traverse[List] with Alternative[List] with Monad[List] with CoflatMap[List] with Align[List] {

I'm not sure why in alleycats these instances were separated in the first place. Perhaps, it was just an oversight. I seems that if we merged them in just one implementor, then it would solve the issue. Perhaps, there were a reason to keep them separated, but I feel it may make sense to give it a shot and merge them. And if the effort fails for some reason, it would be nice to leave a comment about it in the sources, because for now it is not obvious at all.

Regarding the binary compatibility issue:
In the case when an implicit val is intentionally removed from a class or trait (and Mima apparently does not like it), the simplest way to overcome that is to remove implicit specifier from the val and furthermore make that val package-private. It should make Mima pretty happy. In other words, do not remove the implicit val physically, but rather un-implicit it and hide.

@LaurenceWarne
Copy link
Author

In theory, all these instances (i.e., Traverse, Monad, Alternative, etc) can be (and usually are) gathered in a single implementation class and therefore exposed by a single implicit val. For instance, see

implicit val catsStdInstancesForList
: Traverse[List] with Alternative[List] with Monad[List] with CoflatMap[List] with Align[List] =
new Traverse[List] with Alternative[List] with Monad[List] with CoflatMap[List] with Align[List] {

I'm not sure why in alleycats these instances were separated in the first place. Perhaps, it was just an oversight. I seems that if we merged them in just one implementor, then it would solve the issue. Perhaps, there were a reason to keep them separated, but I feel it may make sense to give it a shot and merge them. And if the effort fails for some reason, it would be nice to leave a comment about it in the sources, because for now it is not obvious at all.

Makes sense, and FWIW moving all to one implementor doesn't appear to cause any test failures.

Regarding the binary compatibility issue: In the case when an implicit val is intentionally removed from a class or trait (and Mima apparently does not like it), the simplest way to overcome that is to remove implicit specifier from the val and furthermore make that val package-private. It should make Mima pretty happy. In other words, do not remove the implicit val physically, but rather un-implicit it and hide.

I may have misunderstood 😅 , but after moving to a common implementer and changing the old implicits:

trait SetInstances {
  private[std] val alleyCatsSetTraverse: Traverse[Set] = alleycatsStdInstancesForSet
  private[std] val alleyCatsStdSetMonad: Monad[Set] with Alternative[Set] = alleycatsStdInstancesForSet
  private[std] val alleyCatsSetTraverseFilter: TraverseFilter[Set] = alleycatsStdInstancesForSet

  implicit val alleycatsStdInstancesForSet
    : Monad[Set] with Alternative[Set] with Traverse[Set] with TraverseFilter[Set] = ...

I run into a few different mima issues:

[error]  * abstract synthetic method alleycats$std$SetInstances$_setter_$alleycatsStdInstancesForSet_=(cats.Monad)Unit in interface alleycats.std.SetInstances is present only in current version
[error]    filter with: ProblemFilters.exclude[ReversedMissingMethodProblem]("alleycats.std.SetInstances.alleycats$std$SetInstances$_setter_$alleycatsStdInstancesForSet_=")
[error]  * abstract method alleycatsStdInstancesForSet()cats.Monad in interface alleycats.std.SetInstances is present only in current version
[error]    filter with: ProblemFilters.exclude[ReversedMissingMethodProblem]("alleycats.std.SetInstances.alleycatsStdInstancesForSet")
[error]  * static method alleyCatsSetTraverseFilter()cats.TraverseFilter in class alleycats.std.all does not have a correspondent in current version
[error]    filter with: ProblemFilters.exclude[DirectMissingMethodProblem]("alleycats.std.all.alleyCatsSetTraverseFilter")
[error]  * static method alleyCatsSetTraverse()cats.Traverse in class alleycats.std.all does not have a correspondent in current version
[error]    filter with: ProblemFilters.exclude[DirectMissingMethodProblem]("alleycats.std.all.alleyCatsSetTraverse")
[error]  * static method alleyCatsStdSetMonad()cats.Monad in class alleycats.std.all does not have a correspondent in current version
[error]    filter with: ProblemFilters.exclude[DirectMissingMethodProblem]("alleycats.std.all.alleyCatsStdSetMonad")
[error]  * static method alleyCatsSetTraverseFilter()cats.TraverseFilter in class alleycats.std.set does not have a correspondent in current version
[error]    filter with: ProblemFilters.exclude[DirectMissingMethodProblem]("alleycats.std.set.alleyCatsSetTraverseFilter")
[error]  * static method alleyCatsSetTraverse()cats.Traverse in class alleycats.std.set does not have a correspondent in current version
[error]    filter with: ProblemFilters.exclude[DirectMissingMethodProblem]("alleycats.std.set.alleyCatsSetTraverse")
[error]  * static method alleyCatsStdSetMonad()cats.Monad in class alleycats.std.set does not have a correspondent in current version
[error]    filter with: ProblemFilters.exclude[DirectMissingMethodProblem]("alleycats.std.set.alleyCatsStdSetMonad")

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.

2 participants