Skip to content

Fix logic when comparing var/def bindings with val refinements #18049

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

Merged
merged 2 commits into from
Jun 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 15 additions & 4 deletions compiler/src/dotty/tools/dotc/core/TypeComparer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1971,7 +1971,7 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
def tp1IsSingleton: Boolean = tp1.isInstanceOf[SingletonType]

// A relaxed version of isSubType, which compares method types
// under the standard arrow rule which is contravarient in the parameter types,
// under the standard arrow rule which is contravariant in the parameter types,
// but under the condition that signatures might have to match (see sigsOK)
// This relaxed version is needed to correctly compare dependent function types.
// See pos/i12211.scala.
Expand All @@ -1988,10 +1988,21 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
case _ => inFrozenGadtIf(tp1IsSingleton) { isSubType(info1, info2) }

def qualifies(m: SingleDenotation): Boolean =
val info1 = m.info.widenExpr
isSubInfo(info1, tp2.refinedInfo.widenExpr, m.symbol.info.orElse(info1))
val info2 = tp2.refinedInfo
val isExpr2 = info2.isInstanceOf[ExprType]
val info1 = m.info match
case info1: ValueType if isExpr2 || m.symbol.is(Mutable) =>
// OK: { val x: T } <: { def x: T }
// OK: { var x: T } <: { def x: T }
// NO: { var x: T } <: { val x: T }
ExprType(info1)
case info1 @ MethodType(Nil) if isExpr2 && m.symbol.is(JavaDefined) =>
// OK{ { def x(): T } <: { def x: T} // if x is Java defined
ExprType(info1.resType)
case info1 => info1
isSubInfo(info1, info2, m.symbol.info.orElse(info1))
|| matchAbstractTypeMember(m.info)
|| (tp1.isStable && isSubType(TermRef(tp1, m.symbol), tp2.refinedInfo))
|| (tp1.isStable && m.symbol.isStableMember && isSubType(TermRef(tp1, m.symbol), tp2.refinedInfo))

tp1.member(name) match // inlined hasAltWith for performance
case mbr: SingleDenotation => qualifies(mbr)
Expand Down
7 changes: 7 additions & 0 deletions tests/neg/i13703.check
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,10 @@
| ^^^^^^^^^^
| refinement cannot be a mutable var.
| You can use an explicit getter i and setter i_= instead
-- [E007] Type Mismatch Error: tests/neg/i13703.scala:5:78 -------------------------------------------------------------
5 |val f2: Foo { val i: Int; def i_=(x: Int): Unit } = new Foo { var i: Int = 0 } // error
| ^
| Found: Object with Foo {...}
| Required: Foo{val i: Int; def i_=(x: Int): Unit}
|
| longer explanation available when compiling with `-explain`
4 changes: 3 additions & 1 deletion tests/neg/i13703.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,6 @@ trait Foo extends reflect.Selectable

val f: Foo { var i: Int } = new Foo { var i: Int = 0 } // error

val f2: Foo { val i: Int; def i_=(x: Int): Unit } = new Foo { var i: Int = 0 } // OK
val f2: Foo { val i: Int; def i_=(x: Int): Unit } = new Foo { var i: Int = 0 } // error

val f3: Foo { def i: Int; def i_=(x: Int): Unit } = new Foo { var i: Int = 0 } // OK
15 changes: 15 additions & 0 deletions tests/neg/i18047.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
def foo(x: Any { def foo: Int }): Any { val foo: Int } = x // error
def foo1(x: Any { val foo: Int }): Any { def foo: Int } = x // ok
def foo2(x: Any { val foo: Int }): Any { val foo: Int } = x // ok
def foo3(x: Any { def foo: Int }): Any { def foo: Int } = x // ok

class Foo:
val foo: Int = 1
class Foo1:
def foo: Int = 1
class Foo2:
var foo: Int = 1

def foo4(x: Foo): Any { val foo: Int } = x // ok
def foo4(x: Foo1): Any { val foo: Int } = x // error
def foo4(x: Foo2): Any { val foo: Int } = x // error
2 changes: 1 addition & 1 deletion tests/neg/i4496b.scala
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ object TestStructuralVar {
type T = {val a: Int; def a_=(x: Int): Unit}
def upcast1(v: Foo1): T = v // error
def upcast2(v: Foo2): T = v // error
def upcast3(v: Foo3): T = v
def upcast3(v: Foo3): T = v // error
def verify(v: T) = ()
def test(): Unit = {
verify(upcast1(new Foo1 { val a = 10 }))
Expand Down
4 changes: 2 additions & 2 deletions tests/run/i4496a.scala
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ class Foo3 { var a: Int = 10 }
object Test {
def main(args: Array[String]): Unit = {
assert((new Foo1 : {val a: Int}).a == 10)
assert((new Foo2 : {val a: Int}).a == 10)
assert((new Foo3 : {val a: Int}).a == 10)
assert((new Foo2 : {def a: Int}).a == 10)
assert((new Foo3 : {def a: Int}).a == 10)
}
}
31 changes: 20 additions & 11 deletions tests/run/i4496b.scala
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@ object Test {

// Consider one module upcasting all these instances to T. These casts are clearly well-typed.
type T = {val a: Int}
type T2 = {def a: Int}
def upcast1(v: Foo1): T = v
def upcast2(v: Foo2): T = v
def upcast3(v: Foo3): T = v
def upcast2(v: Foo2): T2 = v
def upcast3(v: Foo3): T2 = v

// These accesses are also clearly well-typed
def consume(v: T) = v.a
Expand All @@ -31,24 +32,32 @@ object Test {
assert(v.a == 10)
}

def consume2(v: T2) = v.a
inline def consumeInl2(v: T2) = v.a
def verify2(v: T2) = {
assert(consume2(v) == 10)
assert(consumeInl2(v) == 10)
assert(v.a == 10)
}

def test(): Unit = {
// These calls are also clearly well-typed, hence can't be rejected.
verify(upcast1(new Foo1 { val a = 10 }))
verify(upcast2(new Foo2 { val a = 10 }))
verify(upcast3(new Foo3 { var a = 10 }))
verify2(upcast2(new Foo2 { val a = 10 }))
verify2(upcast3(new Foo3 { var a = 10 }))
// Ditto, so we must override access control to the class.
verify(upcast1(new FooBar1))
verify(upcast2(new FooBar2))
verify(upcast3(new FooBar3))
verify2(upcast2(new FooBar2))
verify2(upcast3(new FooBar3))

// Other testcases
verify(new {val a = 10} : T)
verify(new {var a = 10} : T)
verify(new {def a = 10} : T)
verify2(new {var a = 10} : T2)
verify2(new {def a = 10} : T2)

verify(new Bar1 : T)
verify(new Bar2 : T)
verify(new Bar3 : T)
verify2(new Bar2 : T2)
verify2(new Bar3 : T2)
}
}

Expand Down Expand Up @@ -85,7 +94,7 @@ object Test {
}

object TestStructuralVar {
type T = {val a: Int; def a_=(x: Int): Unit}
type T = {def a: Int; def a_=(x: Int): Unit}
def upcast3(v: Foo3): T = v
def consume(v: T) = v.a
inline def consumeInl(v: T) = v.a
Expand Down