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

Also consider private symbols in implicit scope of type #14054

Merged
merged 2 commits into from Dec 17, 2021

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Dec 6, 2021

Fixes #14013

@odersky
Copy link
Contributor Author

odersky commented Dec 6, 2021

test performance please

@dottybot
Copy link
Member

dottybot commented Dec 6, 2021

performance test scheduled: 1 job(s) in queue, 0 running.

@dottybot
Copy link
Member

dottybot commented Dec 6, 2021

Performance test finished successfully:

Visit https://dotty-bench.epfl.ch/14054/ to see the changes.

Benchmarks is based on merging with master (097356d)

@odersky
Copy link
Contributor Author

odersky commented Dec 6, 2021

@anatoliykmetyuk Before merging this, I'd like to see what it does for the other benchmark tests, but they have not been running for a while.

@odersky
Copy link
Contributor Author

odersky commented Dec 13, 2021

test performance please

@dottybot
Copy link
Member

performance test scheduled: 1 job(s) in queue, 0 running.

@dottybot
Copy link
Member

Performance test finished successfully:

Visit https://dotty-bench.epfl.ch/14054/ to see the changes.

Benchmarks is based on merging with master (0367d6b)

@odersky
Copy link
Contributor Author

odersky commented Dec 13, 2021

Seems there is no significant slowdown. re2 did not work yet, but that should not exercise the functionality anyway.

Copy link
Contributor

@nicolasstucki nicolasstucki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise LGTM

object Foo {
case class Bar(i: Int)

private implicit class BarOps(bar: Bar) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was marked as scala2-compat, is this behavior something we also want with givens?

Should this one be visible in class Foo?

private given BarOps: AnyRef with {
    extension (bar: Bar)
      def twice: Bar = Bar(bar.i * 2)
  }

@odersky
Copy link
Contributor Author

odersky commented Dec 16, 2021

@nicolasstucki yes, we want it for givens as well. I can add a test.

@odersky odersky merged commit 7a47334 into scala:master Dec 17, 2021
@odersky odersky deleted the fix-14013 branch December 17, 2021 13:01
@Kordyjan Kordyjan added this to the 3.1.2 milestone Aug 2, 2023
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.

Scala 2 regression: Private implicit conversion in companion object scope is not found inside companion class
5 participants