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

Prefer type of overridden member when inferring (under -Xsource:3) #9891

Merged
merged 1 commit into from Feb 9, 2022

Conversation

som-snytt
Copy link
Contributor

@som-snytt som-snytt commented Jan 5, 2022

Under -Xsource:3, adopt the new rule that the type of an override is inferred from the overridden member, and not from the right-hand side of the overriding element, which may narrow the type unexpectedly. The type can be narrowed by supplying an explicit type.

Fixes scala/bug#7212

@scala-jenkins scala-jenkins added this to the 2.13.9 milestone Jan 5, 2022
@scala scala deleted a comment from som-snytt Jan 19, 2022
@SethTisue SethTisue added the release-notes worth highlighting in next release notes label Jan 19, 2022
@SethTisue
Copy link
Member

CI likes it now. Is it ready for review? I'm in favor of the change, given that it's behind -Xsource:3.

@som-snytt
Copy link
Contributor Author

I don't remember what I wasn't sure about. If it breaks something, we might finally get a sense of who is actually using -Xsource:3 or, as Belle says in the current movie, How can we find one person among 5 billion?

@som-snytt som-snytt marked this pull request as ready for review January 19, 2022 03:55
@SethTisue
Copy link
Member

SethTisue commented Jan 19, 2022

Mind supplying a nice user-facing PR description?

@SethTisue
Copy link
Member

SethTisue commented Feb 2, 2022

Final review by @lrytz, please — I just want to be sure you're comfortable with the change.

If merged, this increases the pressure on us to produce a really nice doc page that documents everything that -Xsource:3 changes.

@SethTisue SethTisue changed the title Prefer type of overridden member when inferring Prefer type of overridden member when inferring (under -Xsource:3) Feb 2, 2022
Copy link
Member

@lrytz lrytz left a comment

Choose a reason for hiding this comment

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

👍 LGTM

@SethTisue
Copy link
Member

SethTisue commented Sep 22, 2022

we might finally get a sense of who is actually using -Xsource:3

yup, that is now happening :-)

e.g. scala/bug#12645 , and also:

https://twitter.com/not_xuwei_k/status/1572987057621929984

hold up. given:

  • you want to keep bincompat in both Scala 2 and 3
  • forgot a type ascription
  • already released using "-Xsource:3"

since Scala 2 and 3 are already published on different types, it would be super difficult to bump to Scala 2.13.9
(what am I gonna do with json4s?)

bumping to 2.13.9 without type ascriptions, mima yells at me for changing return types in Scala 2, and if I did add it mima yells at me for changing return types in Scala 3

there's non-zero chance that dropping "-Xsource:3" would work, but likely a checkmate for bincompat anyway

@SethTisue
Copy link
Member

SethTisue commented Sep 22, 2022

@xuwei-k perhaps you'll need to resort to version-specific sources. either duplicate the entire file, or if that's too much duplication, make the return type a type alias and define the type alias differently in version-specific sources

@som-snytt
Copy link
Contributor Author

som-snytt commented Sep 22, 2022

Minimally, it ought to warn under -Xmigration. The purpose of -Xsource;3 is to aid in migration to 3, as well as leverage Scala 3ish features. The other example was implicit lookup, a major feature, which was made opt-in (because it wasn't quite ready for prime time). Maybe there's another mechanism to make it easier to use.

The other footnote is that they say don't rely on inferred types for API.

@SethTisue
Copy link
Member

I'm curious when/where this design decision was made on the Scala 3 side — does anyone know of tickets, discussions, etc?

@SethTisue
Copy link
Member

there is some talk now on scala/bug#12671 that this may need to be refined further to match the Scala 3 behavior more closely

@hmemcpy
Copy link

hmemcpy commented Dec 20, 2022

Sorry to necromance this. I just tried to upgrade our 2.13.6 project straight to 2.13.10 (we're using -Xsource:3), and I have some errors relating to this issue, as well as (likely) #10012. I'm still trying to work around it. If those are bugs, I'll try to minimize and report them.

@som-snytt
Copy link
Contributor Author

@hmemcpy Thanks for the upgrade effort. It's not necessary to announce on an old PR. Since twitter is moribund, there is discord or the contributors forum to open a topic about this upgrade challenge. A fresh ticket for bugs is welcome, of course.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes worth highlighting in next release notes
Projects
None yet
5 participants