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

Remove by-name restriction for case copy #10770

Merged
merged 1 commit into from May 22, 2024

Conversation

som-snytt
Copy link
Contributor

@som-snytt som-snytt commented May 2, 2024

-Xsource-features:case-copy-by-name synthesizes case copy method when a parameter is by-name.

Fixes scala/bug#7879

@scala-jenkins scala-jenkins added this to the 2.13.15 milestone May 2, 2024
@som-snytt som-snytt force-pushed the issue/7879-case-copy-by-name branch from 6b329bf to 7b8b267 Compare May 3, 2024 15:09
@lrytz
Copy link
Member

lrytz commented May 6, 2024

putting behind -Xsource-features

I think we have to as it's changing the binary output.

@som-snytt som-snytt force-pushed the issue/7879-case-copy-by-name branch from 7b8b267 to 914c657 Compare May 8, 2024 18:53
@som-snytt som-snytt marked this pull request as ready for review May 8, 2024 18:59
@som-snytt som-snytt force-pushed the issue/7879-case-copy-by-name branch 2 times, most recently from 4e5ceaa to 9e19996 Compare May 13, 2024 12:21
@som-snytt
Copy link
Contributor Author

Refactored copyOK to use braces and isAllowed instead of isDisallowed. Added a neg test for varargs. (There is no existing test.)

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.

otherwise LGTM

else {
val classParamss = constrParamss(cdef)
def copyOK = {
def warn() = runReporting.warning(cdef.namePos, "case `copy` method is allowed to have by-name parameters under Scala 3 (or with -Xsource-features:case-copy-by-name)", Scala3Migration, cdef.symbol)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
def warn() = runReporting.warning(cdef.namePos, "case `copy` method is allowed to have by-name parameters under Scala 3 (or with -Xsource-features:case-copy-by-name)", Scala3Migration, cdef.symbol)
def warn() = if (currentRun.isScala3) runReporting.warning(cdef.namePos, "case `copy` method is allowed to have by-name parameters under Scala 3 (or with -Xsource-features:case-copy-by-name)", Scala3Migration, cdef.symbol)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added test

@som-snytt som-snytt force-pushed the issue/7879-case-copy-by-name branch from 9e19996 to 09559f3 Compare May 21, 2024 16:05
@som-snytt som-snytt requested a review from lrytz May 21, 2024 16:07
@lrytz lrytz merged commit 43fab32 into scala:2.13.x May 22, 2024
3 checks passed
@som-snytt som-snytt deleted the issue/7879-case-copy-by-name branch May 22, 2024 07:42
@SethTisue SethTisue added the release-notes worth highlighting in next release notes label May 22, 2024
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
4 participants