Skip to content

Use normal mixed-in Product.productIterator#10002

Merged
lrytz merged 2 commits into
scala:2.13.xfrom
som-snytt:tweak/untyped-product-iterator
Apr 19, 2022
Merged

Use normal mixed-in Product.productIterator#10002
lrytz merged 2 commits into
scala:2.13.xfrom
som-snytt:tweak/untyped-product-iterator

Conversation

@som-snytt

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

@joroKr21 joroKr21 left a comment

Copy link
Copy Markdown
Member

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor Author

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

@lrytz lrytz left a comment

Copy link
Copy Markdown
Member

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
Copy Markdown
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

Development

Successfully merging this pull request may close these issues.

5 participants