Navigation Menu

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

Fix for SI-6663 #1619

Closed
wants to merge 2 commits into from
Closed

Fix for SI-6663 #1619

wants to merge 2 commits into from

Conversation

jannic
Copy link
Contributor

@jannic jannic commented Nov 14, 2012

The first commit fixes the bug reported in SI-6663, the second one another similar case as noted in a comment on that bug report.

I'd guess that there are other possible contents of cxTree not yet handled. Is there a better way to fix this than to add all such cases manually to mkInvoke?

Fix mkInvoke to handle selectDynamic calls of the form

  new C.foo[T].xyz

(where C extends Dynamic)

Without this patch, the type parameter was silently ignored, and
possibly inferred to a different.

This patch fixes mkInvoke to handle that case, where the ctxTree has the
form Select(TypeApply(fun, targs), nme)
Fix mkInvoke to handle selectDynamic calls of the form

new C.foo[T].xyz :U

(where C extends Dynamic)

Without this patch, the type parameter T was silently ignored, and
possibly inferred to a different.

This patch fixes mkInvoke to handle that case, where the ctxTree has
the form Typed(...)
@scala-jenkins
Copy link

Started jenkins job pr-rangepos at https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/940/

@scala-jenkins
Copy link

jenkins job pr-rangepos: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-rangepos/940/

@scala-jenkins
Copy link

Started jenkins job pr-scala-testsuite-linux-opt at https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/1647/

@scala-jenkins
Copy link

jenkins job pr-scala-testsuite-linux-opt: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/1647/

@jsuereth
Copy link
Member

I think this should target 2.10.x

@jannic jannic closed this Nov 14, 2012
@jannic jannic reopened this Nov 14, 2012
@jannic
Copy link
Contributor Author

jannic commented Nov 14, 2012

Oops sorry - didn't mean to click on 'close'.

@jsuereth: anything I can do regarding 2.10.x? Check if patch applies cleanly? Switch pull request to different branch?

@retronym
Copy link
Member

Yes, please close this PR and reopen against 2.10.x.

Review by @namin

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants