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

Allow package private classes to change binary API #257

Closed
wants to merge 5 commits into from
Closed

Allow package private classes to change binary API #257

wants to merge 5 commits into from

Conversation

dwijnand
Copy link
Collaborator

Refs #53

@SethTisue
Copy link
Collaborator

is there substantial copy-and-paste here as you'd feared earlier today, or did it end up being fresh code?

if this PR opens up a whole new space of possibilities for MiMa features, as we discussed, then some coordinated doc updates (maybe even just a paragraph or two in the README?) might be appropriate to include?

@dwijnand
Copy link
Collaborator Author

I stripped scalac 2.10's UnPickler, which (I realised after submitting) won't work against scalac 2.11/2.12.

This adds a reasonable start for extracting more out of the pickler info (as in you need to get it to pull out more things for you) but it doesn't, for instance, even close #53 because it only does the class, not its methods...

@lrytz
Copy link
Contributor

lrytz commented Oct 29, 2018

I'm not sure we should go in that direction (starting to read scala signatures).

Somewhat related: it's really difficult to create a correct mapping between the symbol table and the classfile level (name-mangled method names, anonymous classes, etc). This was the major source of issues for the Scala inliner before 2.12.

I think MiMa should only work on the bytecode. However, we could think of new channels to pass information from the compiler to MiMa. For example we could have make the Scala compiler emit annotations on methods that were made public by the compiler. We could also have an annotation in the standard library that people could use for classes/members that MiMa should ignore (an alternative to the mima config - maybe easier to use/manage?).

Disclaimer: I don't know enough about how MiMa works in detail, I might be missing something! What kind of exceptions does it currently have, for example to not fail for anonymous function classes, or foo$anonfun methods?

@dwijnand
Copy link
Collaborator Author

I'm keen to have a solution for current versions of Scala. If we want to add a more robust, or in other ways better, solution in future Scala, then we can migrate future MiMa to that. But I'm guessing reading scala signatures is the best we can do right now (outside of just not ship the features/enhancements).

Of course, I'm happy to find a happy compromise on how it's implemented (e.g how much to re-use or not re-use out of scalac, and what to duplicate in MiMa, how to handle scalac differences, etc).

If it's easier we can jump in a call to discuss, once I'm back in the office.

@dwijnand
Copy link
Collaborator Author

dwijnand commented Nov 8, 2018

This change is a hacky rush, needs fixing to land, and there's some concern.

I don't have the bandwidth to land it.

@dwijnand dwijnand closed this Nov 8, 2018
@dwijnand dwijnand deleted the package-private branch November 8, 2018 23:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants