-
Notifications
You must be signed in to change notification settings - Fork 1.1k
checkNoPrivateLeaks: Do not allow types to refer to leaky aliases #2189
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
Conversation
This PR is not going to be checked in the CI until we fix the signage issue of |
What issue? Do I just need to resign the .drone.yml? |
Yes, that looks like it's the issue. |
I'm not sure why - maybe it was how/because we restarted the server. |
Nope, when I resign it, it's identical to the existing one. |
Put this:
|
@felixmulder You should be able to push to this branch an extra commit with that. |
Or even better, make a separate PR and merge if the tests start. |
Hmm, I had a slight modification to my But anyway, let's try to fix this using IM. |
Fixed by restarting drone. |
Hooray, we did it! |
`checkNoPrivateLeaks` can force a lot of things, this lead to hard-to-reproduce issues in unpickling because we called `checkNoPrivateLeaks` on the type parameters of a class before anything in the class was indexed. We fix this by making sure that `checkNoPrivateLeaks` never transforms type symbols, only term symbols, therefore we can unpickle type parameters without forcing too many things. tests/neg/leak-type.scala illustrates the new restriction that this necessitates.
552cb55
to
6ae376a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Glad it was relatively straightforward in the end.
@smarter that fixed it! Thanks 😄 |
checkNoPrivateLeaks
can force a lot of things, this lead tohard-to-reproduce issues in unpickling because we called
checkNoPrivateLeaks
on the type parameters of a class before anythingin the class was indexed. We fix this by making sure that
checkNoPrivateLeaks
never transforms type symbols, only term symbols,therefore we can unpickle type parameters without forcing too many
things.
tests/neg/leak-type.scala
illustrates the new restriction thatthis necessitates.
@OlivierBlanvillain Can you confirm that this fixes the issue for you too?