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

Take care of all the missing outer pointer in Scala 2 library #22640

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

hamzaremmal
Copy link
Member

Now that SIP-52 was approved, we can use it to hide constructors that would have been generated by nsc but not dotc.

@sjrd
Copy link
Member

sjrd commented Feb 22, 2025

I don't understand why we need these @publicInBinary definitions. All the introduced constructors are in private classes. That means no call to those constructors could ever exist in another compilation unit.

IMO for these cases we should keep the MiMa filters instead, with a comment for that section saying "constructors of private classes, not an issue".


For the two private val _ = Enclosing.this in objects, that seems warranted. However :

  1. It would be good to first have a commit with a verbatim copy of the file upstream.
  2. Then a commit that actually introduces the patch and removes the MiMa filter.

Otherwise it's very tricky to determine now, and in the future, what changed.

Moreover, these lines really deserve comments explaining why they are there.

@hamzaremmal hamzaremmal disabled auto-merge February 22, 2025 16:10
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