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

Use normal mixed-in Product.productIterator #10002

Merged
merged 2 commits into from Apr 19, 2022

Conversation

som-snytt
Copy link
Contributor

Omit generating the method and acquire the usual
trait implementation. This follows Scala 3.

(The special runtime support was added as an experiment
and never removed when the experiment ended.)

@scala-jenkins scala-jenkins added this to the 2.13.9 milestone Apr 13, 2022
@SethTisue SethTisue marked this pull request as draft April 13, 2022 19:47
Omit generating the method and acquire the usual
trait implementation. This follows Scala 3.

(The special runtime support was added as an experiment
and never removed when the experiment ended.)
@som-snytt som-snytt force-pushed the tweak/untyped-product-iterator branch from 384719f to 3cc2561 Compare April 14, 2022 20:19
@som-snytt som-snytt added the internal not resulting in user-visible changes (build changes, tests, internal cleanups) label Apr 14, 2022
@som-snytt som-snytt marked this pull request as ready for review April 14, 2022 21:02
@som-snytt
Copy link
Contributor Author

Was going to skip deprecating the utility method in ScalaRunTime, but maybe a comment would assist someone cleaning up later in Scala 3.

@joroKr21
Copy link
Member

I remember wondering why this method is generated. I guess it's one of those things that everyone assumed there is a good reason when there is none. 👍 maybe delete instead of commenting out?

@som-snytt
Copy link
Contributor Author

Yes, I meant to comment that I followed local code style in commenting out.

I mentioned on Scala 3 that Scala 3 does _1 components! I didn't try uncommenting that code.

Copy link
Member

@joroKr21 joroKr21 left a comment

Choose a reason for hiding this comment

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

Either way I'm in favour of this change

@som-snytt
Copy link
Contributor Author

I will now follow up by deleting just the rogue iterator that is commented out here. In part to placate @joroKr21

@som-snytt
Copy link
Contributor Author

I was about to add the deprecation of the runtime method, which we can't delete because of compatibility.

But unbootstrapped build would complain. Yet perhaps I will suppress it.

[warn] .../scala/src/reflect/scala/reflect/internal/Types.scala:1136:15: method typedProductIterator in object ScalaRunTime is deprecated (since 2.13.9): Case classes inherit productIterator
[warn]   case object WildcardType extends ProtoType {

@som-snytt
Copy link
Contributor Author

I think there should be a chat channel just for -Wconf questions where Lukas gets notifications on his phone.

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. Thanks to the mixin forwarder, case classes still have a productIterator method in bytecode, so there's no change for the runtime-generated SerialVersionUIDs.

@SethTisue
Copy link
Member

MiMA user confused by the effect of this: lightbend-labs/mima#723

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal not resulting in user-visible changes (build changes, tests, internal cleanups)
Projects
None yet
5 participants