Skip to content

SI-7655 Enforce stable id patterns conform to scrutinee type #2742

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

Closed
wants to merge 1 commit into from

Conversation

retronym
Copy link
Member

This was only being done if the stable id pattern was a direct
reference to a module.

This commit distinguishes stable ID patterns with the recently
added facility in TreeInfo. (Actually, this might be overkill
in adapt and we could probably get away with just checking for
_: Ident | _: Select.)

Review by @adriaanm.

Opinions on the particular way to check for the stable ID pattern are welcome.

@ghost ghost assigned adriaanm Jul 16, 2013
This was only being done if the stable id pattern was a direct
reference to a module.

This commit distinguishes stable ID patterns with the recently
added facility in TreeInfo. (Actually, this might be overkill
in adapt and we could probably get away with just checking for
`_: Ident | _: Select`.)

It seems, however, from the quantity of changes required to
bootstrap the distro under this regime, that this change
overreaches. We might need to check if the spec is correct here.
We might be able to keep some of the change (such as aliases
for modules such as scala.Right) as an error, and move the rest
under -Xlint.
@retronym
Copy link
Member Author

I'll leave this open so just to get a build. But we can't really merge it as is, based on the quantity of changes needed to bootstrap under this interpretation of the spec.

@retronym
Copy link
Member Author

Okay, that was near enough to a green test run. Closing the PR pending discussion on the the best approach for backwards compatibility / convenience vs safety etc.

@retronym retronym closed this Jul 16, 2013
@@ -188,9 +188,9 @@ object Futures {
})

val partFuns = unsetFts.map((p: Pair[Int, Future[Any]]) => {
val FutCh = p._2.inputChannel
val FutCh = p._2.inputChannel.asInstanceOf[Channel[Any]]
Copy link
Contributor

Choose a reason for hiding this comment

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

doesn't p._2 : Future[Any] imply p._2.inputChannel: Channel[Any]?

abstract class Future[+T] extends Responder[T] with Function0[T] {
// ...
  def inputChannel: InputChannel[T]

Copy link
Member Author

Choose a reason for hiding this comment

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

You're right, that is a left-over from my first round of trying to fix the problem. The actual fix was to use guards.

@adriaanm
Copy link
Contributor

It seems this is one of those occasions where we should change the spec to match the implementation. I expect the stricter version to be pretty painful to migrate to, and not sure about the benefit that it brings, as type information doesn't really tend to tell you much about what equals will say (unfortunately).

@retronym
Copy link
Member Author

Yeah, I agree. The spec is too strict here to be useful.

There is definitely a need for some static checking though. Maybe that will be under Xlint. See SI-7211 for example.

I'll ask Martin for a clarification on this next time I see him.

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