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

SI-8363 Disable -Ydelambdafy:method in constructor position #3616

Merged
merged 1 commit into from Mar 12, 2014

Conversation

Projects
None yet
4 participants
@retronym
Member

retronym commented Mar 10, 2014

As @magarciaEPFL has done in his experimental optimizer [1], we can
avoid running into limitations of lambdalift (either VerifyErrors,
ala SI-6666, or compiler crashes, such as this bug), by using the
old style of "inline" lambda translation when in a super- or self-
construtor call.

We might be able to get these working later on, but for now we
shouldn't block adoption of -Ydelamndafy:method for this corner
case.

[1] https://github.com/magarciaEPFL/scala/blob/GenRefactored99sZ/src/compiler/scala/tools/nsc/transform/UnCurry.scala#L227

Review by @gkossakowski

SI-8363 Disable -Ydelambdafy:method in constructor position
As @magarciaEPFL has done in his experimental optimizer [1], we can
avoid running into limitations of lambdalift (either `VerifyError`s,
ala SI-6666, or compiler crashes, such as this bug), by using the
old style of "inline" lambda translation when in a super- or self-
construtor call.

We might be able to get these working later on, but for now we
shouldn't block adoption of `-Ydelamndafy:method` for this corner
case.

[1] https://github.com/magarciaEPFL/scala/blob/GenRefactored99sZ/src/compiler/scala/tools/nsc/transform/UnCurry.scala#L227
@adriaanm

This comment has been minimized.

Show comment
Hide comment
@adriaanm

adriaanm Mar 10, 2014

Member

spurious fail due to network error in IDE build -- manual rebuild: https://scala-webapps.epfl.ch/jenkins/job/pr-scala-integrate-ide/6668/console

Member

adriaanm commented Mar 10, 2014

spurious fail due to network error in IDE build -- manual rebuild: https://scala-webapps.epfl.ch/jenkins/job/pr-scala-integrate-ide/6668/console

@gkossakowski

This comment has been minimized.

Show comment
Hide comment
@gkossakowski

gkossakowski Mar 10, 2014

Member

LGTM

I thought we did that already. The problem with lambdas in constructors was discovered by @JamesIry a long time ago. We decided that working it around by falling back to old translation scheme was the way to go.

Member

gkossakowski commented Mar 10, 2014

LGTM

I thought we did that already. The problem with lambdas in constructors was discovered by @JamesIry a long time ago. We decided that working it around by falling back to old translation scheme was the way to go.

@retronym retronym added the reviewed label Mar 10, 2014

@retronym

This comment has been minimized.

Show comment
Hide comment
@retronym

retronym Mar 11, 2014

Member

@gkossakowski I think we just forced some tests for SI-6666 to use -Ydelambdafy:inline as the new tree shapes were no longer detected by the "this will cause a VerifyError" diagnostics I added for SI-6666.

Member

retronym commented Mar 11, 2014

@gkossakowski I think we just forced some tests for SI-6666 to use -Ydelambdafy:inline as the new tree shapes were no longer detected by the "this will cause a VerifyError" diagnostics I added for SI-6666.

@gkossakowski

This comment has been minimized.

Show comment
Hide comment
@gkossakowski

gkossakowski Mar 11, 2014

Member

PLS REBUILD/pr-scala@814ecad

Member

gkossakowski commented Mar 11, 2014

PLS REBUILD/pr-scala@814ecad

@gkossakowski

This comment has been minimized.

Show comment
Hide comment
@gkossakowski

gkossakowski Mar 11, 2014

Member

@retronym: ah yes. That's right.

Member

gkossakowski commented Mar 11, 2014

@retronym: ah yes. That's right.

@gkossakowski

This comment has been minimized.

Show comment
Hide comment
@gkossakowski

gkossakowski Mar 11, 2014

Member

In any case, we should merge this once tests pass.

Member

gkossakowski commented Mar 11, 2014

In any case, we should merge this once tests pass.

@scala-jenkins

This comment has been minimized.

Show comment
Hide comment
@scala-jenkins

scala-jenkins Mar 11, 2014

(kitty-note-to-self: ignore 37321743)
🐱 Roger! Rebuilding pr-scala for 814ecad. 🚨

scala-jenkins commented Mar 11, 2014

(kitty-note-to-self: ignore 37321743)
🐱 Roger! Rebuilding pr-scala for 814ecad. 🚨

@retronym retronym added tested and removed needs-attention labels Mar 11, 2014

adriaanm added a commit that referenced this pull request Mar 12, 2014

Merge pull request #3616 from retronym/ticket/8363
SI-8363 Disable -Ydelambdafy:method in constructor position

@adriaanm adriaanm merged commit 80f77cc into scala:master Mar 12, 2014

1 check passed

default pr-scala Took 52 min.
Details

@adriaanm adriaanm modified the milestones: 2.11.0-RC3, 2.11.0-RC2 Mar 18, 2014

@retronym retronym referenced this pull request Apr 29, 2015

Closed

Is PartialFunction a SAM? #504

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment