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

Restricts isInstanceOf[Null] checks #12905

Merged
merged 2 commits into from
Jun 23, 2021
Merged

Restricts isInstanceOf[Null] checks #12905

merged 2 commits into from
Jun 23, 2021

Conversation

Kordyjan
Copy link
Contributor

@Kordyjan Kordyjan commented Jun 22, 2021

Fixes #4004
isInstanceOf[Nothing], isInstanceOf[Null], isInstanceOf[Singleton] checks are always prohibited

@Kordyjan Kordyjan requested a review from smarter June 22, 2021 18:05
@Kordyjan Kordyjan marked this pull request as draft June 22, 2021 18:10
Fixes #4004
`isInstanceOf[Nothing]` checks are always prohibited
`isInstanceOf[Null]` checks are probihibited unless they can be proven at compiletime, then thay are simplified to `true`
@sjrd
Copy link
Member

sjrd commented Jun 22, 2021

unless they can be proven at compiletime, then thay are simplified to true

What's the point of that? Also, x.isInstanceOf[T] is supposed to answer true when x is a non-null instance of T. So when T is Null, it should always be false. (which is why this is supposed to be disallowed)

@Kordyjan
Copy link
Contributor Author

Kordyjan commented Jun 22, 2021

I thought, checking it at compile-time can have some use cases for metaprogramming. @nicolasstucki, what is your opinion?

On an unrelated note, I've forgotten about explicit nulls, which are apparently using Null type in matches. We can solve it by disallowing isInstanceOf[Null] also there or translate it into expr eq null.

@bishabosha
Copy link
Member

bishabosha commented Jun 23, 2021

I thought, checking it at compile-time can have some use cases for metaprogramming. @nicolasstucki, what is your opinion?

On an unrelated note, I've forgotten about explicit nulls, which are apparently using Null type in matches. We can solve it by disallowing isInstanceOf[Null] also there or translate it into expr eq null.

I think that the pattern match case t: Null should also be banned under this change, I only reported the errors in #12697 because it was currently not making a consistent warning with when -Yexplicit-nulls is turned off

@sjrd
Copy link
Member

sjrd commented Jun 23, 2021

Yes, using explicit-nulls doesn't change the fact that x.isInstanceOf[Null] must always return false, and therefore be ruled out as confusing. And so case x: Null => must also be ruled out because it's the same as x.isInstanceOf[Null], which would always be dead code because it's always false.

also add `scala.Singleton` to untestable types
@Kordyjan
Copy link
Contributor Author

I've made isInstanceOf[Null] disallowed everywhere. Also, I've added scala.Singleton to untestable types.

@Kordyjan Kordyjan marked this pull request as ready for review June 23, 2021 13:33
@smarter smarter merged commit 2ccf681 into master Jun 23, 2021
@smarter smarter deleted the i4004 branch June 23, 2021 14:39
@Kordyjan Kordyjan added this to the 3.0.2 milestone Aug 2, 2023
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.

Disallow type Null in a type pattern or isInstanceOf test
4 participants