Skip to content

Do not generate _N members for case classes in stdlib-bootstrapped #18117

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

nicolasstucki
Copy link
Contributor

@nicolasstucki nicolasstucki commented Jul 3, 2023

Follow-up of #17928

@nicolasstucki nicolasstucki marked this pull request as ready for review July 3, 2023 12:16
@nicolasstucki nicolasstucki force-pushed the fix-case-class-product-members-in-stdlib-bootstrapped branch from 2a1ea2d to 600a512 Compare July 3, 2023 12:37
@nicolasstucki nicolasstucki requested a review from smarter July 4, 2023 07:54
@nicolasstucki nicolasstucki force-pushed the fix-case-class-product-members-in-stdlib-bootstrapped branch from 600a512 to 6aa10c2 Compare July 5, 2023 14:44
val paramNames = ctx.owner.owner.info.decls.toList.filter(_.flags.is(ParamAccessor)).map(_.name)
// case N => this.${paramNames(N)}
val cases = paramNames.zip(0.until(arity)).map { (name, i) =>
val sel = This(clazz).select(name, _.info.isParameterless)
Copy link
Member

Choose a reason for hiding this comment

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

Why not directly select the symbol found above instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I followed the previous implementation too closely. Now I am selecting the symbols directly. I also used zipWithIndex instead of the zip with the range.

@smarter smarter assigned nicolasstucki and unassigned smarter Jul 10, 2023
@nicolasstucki nicolasstucki force-pushed the fix-case-class-product-members-in-stdlib-bootstrapped branch from 6aa10c2 to 1fac412 Compare July 10, 2023 12:54
@nicolasstucki nicolasstucki requested a review from smarter July 10, 2023 12:56
@nicolasstucki
Copy link
Contributor Author

@smarter I also updated the MiMaFilters. You should double-check those changes.

@@ -97,6 +92,9 @@ object MiMaFilters {
ProblemFilters.exclude[MissingTypesProblem]("scala.jdk.IntAccumulator"),
ProblemFilters.exclude[MissingTypesProblem]("scala.jdk.LongAccumulator"),
ProblemFilters.exclude[FinalClassProblem]("scala.collection.ArrayOps$ReverseIterator"),
ProblemFilters.exclude[FinalClassProblem]("scala.Tuple1"),
ProblemFilters.exclude[FinalClassProblem]("scala.Tuple2"),
ProblemFilters.exclude[MissingFieldProblem]("scala.Tuple*._*"), // Tuple1._1, Tuple2._1, Tuple2._2
Copy link
Member

Choose a reason for hiding this comment

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

Where does this difference come from?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure if it is a specialization anymore. I will have to investigate further into this one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I recategorized these failures

@nicolasstucki nicolasstucki force-pushed the fix-case-class-product-members-in-stdlib-bootstrapped branch from 1fac412 to c199e45 Compare July 10, 2023 14:34
@nicolasstucki nicolasstucki force-pushed the fix-case-class-product-members-in-stdlib-bootstrapped branch from c199e45 to df3968b Compare July 10, 2023 14:37
@nicolasstucki nicolasstucki requested a review from smarter July 13, 2023 15:29
@smarter smarter merged commit f4aa793 into scala:main Jul 13, 2023
@smarter smarter deleted the fix-case-class-product-members-in-stdlib-bootstrapped branch July 13, 2023 15:34
@Kordyjan Kordyjan added this to the 3.4.0 milestone Dec 20, 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.

3 participants