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

Null <:< "foo" and other problems with Null subtyping (also in -Yexplicit-nulls) #17467

Closed
sjrd opened this issue May 11, 2023 · 0 comments · Fixed by #17470
Closed

Null <:< "foo" and other problems with Null subtyping (also in -Yexplicit-nulls) #17467

sjrd opened this issue May 11, 2023 · 0 comments · Fixed by #17470
Assignees
Milestone

Comments

@sjrd
Copy link
Member

sjrd commented May 11, 2023

Compiler version

3.3.0-RC5

Problem at a glance

Null is considered to be a subtype of anything that widenDealiases to a nullable type.

Spec context

The spec actually says that Null <:< x.type if the underlying type of x is a nullable. But that is supposed to only apply to singleton types of the form path.type. Currently in dotty, we even have absurd things like Null <:< "foo" because "foo" widens to "String" which is nullable.

See https://scala-lang.org/files/archive/spec/2.13/03-types.html#singleton-types

Minimized code

REPL session without -Yexplicit-nulls

$ cs launch scala:3.3.0-RC5
Welcome to Scala 3.3.0-RC5 (1.8.0_362, Java OpenJDK 64-Bit Server VM).
Type in expressions for evaluation. Or try :help.
                                                                                                    
scala> val x: "foo" = null
val x: "foo" = null
                                                                                                    
scala> val y: 5 = null // Type error, as expected
-- [E007] Type Mismatch Error: -------------------------------------------------
1 |val y: 5 = null // Type error, as expected
  |           ^^^^
  |Found:    Null
  |Required: (5 : Int)
  |Note that implicit conversions were not tried because the result of an implicit conversion
  |must be more specific than (5 : Int)
  |
  | longer explanation available when compiling with `-explain`
1 error found
                                                                                                    
scala> val a: "foo" = "foo"
val a: "foo" = foo
                                                                                                    
scala> val b: a.type = null
val b: a.type = null
                                                                                                    
scala> val c: Any = new AnyRef
val c: Any = java.lang.Object@78d73b1b
                                                                                                    
scala> val d: c.type = null
val d: c.type = null
                                                                                                    
scala> val e: AnyVal = 5
val e: AnyVal = 5
                                                                                                    
scala> val f: e.type = null // Type error as expected
-- [E007] Type Mismatch Error: -------------------------------------------------
1 |val f: e.type = null // Type error as expected
  |                ^^^^
  |Found:    Null
  |Required: (e : AnyVal)
  |Note that implicit conversions were not tried because the result of an implicit conversion
  |must be more specific than (e : AnyVal)
  |
  | longer explanation available when compiling with `-explain`
1 error found
                                                                                                    
scala> summon[Null <:< "foo"]
val res0: Null =:= Null = generalized constraint
                                                                                                    
scala> summon[Null <:< c.type]
val res1: Null =:= Null = generalized constraint

scala> class Foo { def bar(): this.type = null }
// defined class Foo

Even under -Yexplicit-nulls, we have issues:

$ cs launch scala:3.3.0-RC5 -- -Yexplicit-nulls
Welcome to Scala 3.3.0-RC5 (1.8.0_362, Java OpenJDK 64-Bit Server VM).
Type in expressions for evaluation. Or try :help.
                                                                                                    
scala> val x: "foo" = null // hey, progress!
-- [E007] Type Mismatch Error: -------------------------------------------------
1 |val x: "foo" = null // hey, progress!
  |               ^^^^
  |Found:    Null
  |Required: ("foo" : String)
  |Note that implicit conversions were not tried because the result of an implicit conversion
  |must be more specific than ("foo" : String)
  |
  | longer explanation available when compiling with `-explain`
1 error found
                                                                                                    
scala> val x: Any = "foo"
val x: Any = foo
                                                                                                    
scala> val y: x.type = null // oops, still broken
val y: x.type = null
                                                                                                    
scala> val a: String | Null = "foo"
val a: String | Null = foo
                                                                                                    
scala> val b: a.type = null // this is broken too
val b: a.type = null
                                                                                                    
scala> summon[Null <:< a.type]
val res0: Null =:= Null = generalized constraint
                                                                                                    
scala> type Foo = a.type
// defined alias type Foo = a.type
                                                                                                    
scala> summon[Null <:< Foo]
val res1: Null =:= Null = generalized constraint

Expectation

Not having Null <:< T for arbitrary Ts that widenDealias to a nullable type. Only allow it for singleton types (in dotc terminology, TermRefs) whose direct underlying type is nullable, as the spec says.

Further, under -Yexplicit-nulls, IMO we should take the opportunity to tighten the spec to remove the weird clause about the underlying type of singleton types.

@sjrd sjrd self-assigned this May 11, 2023
sjrd added a commit to dotty-staging/dotty that referenced this issue May 11, 2023
The Scala language specification has a peculiar clause about the
nullness of singleton types of the form `path.type`. It says that
`Null <:< path.type` if the *underlying* type `U` of `path` is
nullable itself.

The previous implementation of that rule was overly broad, as it
indiscrimately widened all types. This resulted in problematic
subtyping relationships like `Null <:< "foo"`.

We do not widen anymore. Instead, we specifically handle `TermRef`s
of stable members, which are how dotc represents singleton types.
We also have a rule for `Null <:< null`, which is necessary for
pattern matching exhaustivity to keep working in the presence of
nulls.
sjrd added a commit to dotty-staging/dotty that referenced this issue May 17, 2023
The Scala language specification has a peculiar clause about the
nullness of singleton types of the form `path.type`. It says that
`Null <:< path.type` if the *underlying* type `U` of `path` is
nullable itself.

The previous implementation of that rule was overly broad, as it
indiscrimately widened all types. This resulted in problematic
subtyping relationships like `Null <:< "foo"`.

We do not widen anymore. Instead, we specifically handle `TermRef`s
of stable members, which are how dotc represents singleton types.
We also have a rule for `Null <:< null`, which is necessary for
pattern matching exhaustivity to keep working in the presence of
nulls.
sjrd added a commit to dotty-staging/dotty that referenced this issue May 17, 2023
The Scala language specification has a peculiar clause about the
nullness of singleton types of the form `path.type`. It says that
`Null <:< path.type` if the *underlying* type `U` of `path` is
nullable itself.

The previous implementation of that rule was overly broad, as it
indiscrimately widened all types. This resulted in problematic
subtyping relationships like `Null <:< "foo"`.

We do not widen anymore. Instead, we specifically handle `TermRef`s
of stable members, which are how dotc represents singleton types.
We also have a rule for `Null <:< null`, which is necessary for
pattern matching exhaustivity to keep working in the presence of
nulls.
sjrd added a commit to dotty-staging/dotty that referenced this issue Aug 2, 2023
The Scala language specification has a peculiar clause about the
nullness of singleton types of the form `path.type`. It says that
`Null <:< path.type` if the *underlying* type `U` of `path` is
nullable itself.

The previous implementation of that rule was overly broad, as it
indiscrimately widened all types. This resulted in problematic
subtyping relationships like `Null <:< "foo"`.

We do not widen anymore. Instead, we specifically handle `TermRef`s
of stable members, which are how dotc represents singleton types.
We also have a rule for `Null <:< null`, which is necessary for
pattern matching exhaustivity to keep working in the presence of
nulls.
Kordyjan added a commit that referenced this issue Aug 4, 2023
…r explicit nulls. (#17470)

The Scala language specification has a peculiar clause about the
nullness of singleton types of the form `path.type`. It says that `Null
<:< path.type` if the *underlying* type `U` of `path` is nullable
itself.

The previous implementation of that rule was overly broad, as it
indiscrimately widened all types. This resulted in problematic subtyping
relationships like `Null <:< "foo"`.

We do not widen anymore. Instead, we specifically handle `TermRef`s of
stable members, which are how dotc represents singleton types. We also
have a rule for `Null <:< null`, which is necessary for pattern matching
exhaustivity to keep working in the presence of nulls.

---

#### Under explicit nulls, remove the rule Null <:< x.type.

The specified rule that `Null <:< x.type` when the underlying type `U`
of `x` is nullable is dubious to begin with. Under explicit nulls, it
becomes decidedly out of place. We now disable that rule under
`-Yexplicit-nulls`.
@sjrd sjrd added this to the 3.4.0 milestone Aug 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant