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

SI-8217 allow abstract type members in objects #4024

Merged
merged 1 commit into from Oct 4, 2014

Conversation

retronym
Copy link
Member

@retronym retronym commented Oct 1, 2014

Previously, abstract type members were allowed in objects only when inherited,
but not when declared directly. This inconsistency was not intended. In dotty,
abstract type members are allowed in values and represent existentials; so upon
discussion, it was decided to fix things to conform to dotty and allow such type
members. Adriaan also asked to keep rejecting abstract type members in methods
even though they would conceivably make sense.

Discussions happened on #3407, scala/scala-dist#127.
This code is improved from #3442, keeps closer to the current logic, and passes tests.

Existing tests that have been converted to pos tests show that
this works, and a new test has been added to show that local
aliases (ie term-owned) without a RHS are still rejected.

I've just added the new test to @Blaisorblade's PR, #3978.

Review by @adriaanm

I tend to think this one is okay for 2.11.x, as it only loosens
a restriction. That said, it would be no great loss to wait until
2.12.

Previously, abstract type members were allowed in objects only when inherited,
but not when declared directly. This inconsistency was not intended. In dotty,
abstract type members are allowed in values and represent existentials; so upon
discussion, it was decided to fix things to conform to dotty and allow such type
members. Adriaan also asked to keep rejecting abstract type members in methods
even though they would conceivably make sense.

Discussions happened on scala#3407, scala/scala-dist#127.
This code is improved from scala#3442, keeps closer to the current logic, and passes tests.

Existing tests that have been converted to `pos` tests show that
this works, and a new test has been added to show that local
aliases (ie term-owned) without a RHS are still rejected.
@Blaisorblade
Copy link
Contributor

Thanks for the test @retronym — this looks good 👍 !

@scala-jenkins scala-jenkins added this to the 2.11.3 milestone Oct 1, 2014
@adriaanm
Copy link
Contributor

adriaanm commented Oct 1, 2014

LGTM, including the milestone

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