Skip to content

SI-7753 InstantiateDependentMap narrows type of unstable args #3507

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 6 commits into from

Conversation

adriaanm
Copy link
Contributor

[Most of this comment and the initial fix were implemented by Jason Zaugg.
I just cleaned it up a bit.]

After a soundness fix in SI-3873, instantiation of dependent
method type results behaved differently depending on whether the argument
from which we were propagating information had a stable type
or not. This is particular to substitution into singleton types
over the parameter in question.

If the argument was stable, it was substituted into singleton
types, such as the one below in the prefix in a.type#B
(which is the longhand version of a.B)

scala> class A { type B >: Null <: AnyRef }
defined class A

scala> object AA extends A { type B = String }
defined object AA

scala> def foo(a: A): a.B = null
foo: (a: A)a.B

scala> foo(AA)
res0: AA.B = null

But what if it isn't stable?

scala> foo({def a = AA; a: A { type B <: String}})
res1: a.B = null

This commit changes that to:

scala> foo({def a = AA; a: A { type B <: String}})
res1: A{type B <: String}#B = null

review by @retronym

@adriaanm
Copy link
Contributor Author

This continues where #3482 left off.

@paulp
Copy link
Contributor

paulp commented Feb 11, 2014

No, I don't mean "fix it", I mean put it in as a neg test which should eventually be pos.

){converted.instance.Out};
*/

val v2: Int = indirect(conv(foo))(23) // Fails.
Copy link
Contributor

Choose a reason for hiding this comment

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

You might want to rebase the comment out as it's a bit misleading now that this is a pos test.

@paulp
Copy link
Contributor

paulp commented Feb 11, 2014

I'll send you a PR. That test is freaking poetry.

@paulp
Copy link
Contributor

paulp commented Feb 11, 2014

#3508

@adriaanm adriaanm mentioned this pull request Feb 11, 2014
@adriaanm
Copy link
Contributor Author

test comment improved

We've started calling this method during higher-kinded subtyping
to ensure that variances of higher order type params in overriding
as soundly aligned.

Turns out that running this over the expansion of the SBT task
macro leads to a SOE due to a corrupt owner chain.

I've fixed that in SBT (sbt/sbt#1113),
but we ought not crash like this.

This commit considers NoSymbol to be its own enclosing member and
logs a -Xdev warning. This is analagous to the handling of
`NoSymbol.owner`.
@retronym
Copy link
Member

LGTM.

I noticed the change when adapting Slick to work with
Scala 2.11 in `AbstractSourceCodeGenerator.scala`.
The behaviour changed in a70c821.

This commit locks down the new, correct behaviour with a test.
@adriaanm
Copy link
Contributor Author

Will rebase on master once #3516 is merged

@adriaanm adriaanm closed this Feb 12, 2014
adriaanm and others added 4 commits February 12, 2014 10:39
 SI-8177 co-evolve more than just RefinedTypes
[Most of this comment and the initial fix were implemented by Jason Zaugg.
I just cleaned it up a bit.]

After a soundness fix in SI-3873, instantiation of dependent
method type results behaved differently depending on whether the argument
from which we were propagating information had a stable type
or not. This is particular to substitution into singleton types
over the parameter in question.

If the argument was stable, it was substituted into singleton
types, such as the one below in the prefix in `a.type#B`
(which is the longhand version of `a.B`)

  scala> class A { type B >: Null <: AnyRef }
  defined class A

  scala> object AA extends A { type B = String }
  defined object AA

  scala> def foo(a: A): a.B = null
  foo: (a: A)a.B

  scala> foo(AA)
  res0: AA.B = null

But what if it isn't stable?

  scala> foo({def a = AA; a: A { type B <: String}})
  res1: a.B = null

This commit changes that to:

  scala> foo({def a = AA; a: A { type B <: String}})
  res1: A{type B <: String}#B = null
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.

5 participants